Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

e2e: update e2e tests to use -label-filter #1866

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hj-johannes-lee
Copy link
Contributor

@hj-johannes-lee hj-johannes-lee commented Oct 4, 2024

fixes: #1862
Current system with FOCUS and SKIP is complicated. Introduce label filter, update plain text labels of e2e tests to ginkgo.Label(), and update Makefile.

In addition, put labels to admission webhooks for fpga and sgx.

Current system with FOCUS and SKIP is complicated. Introduce label
filter, update plain text labels of e2e tests to ginkgo>Label(),
and update Makefile.

In addition, put labels to admission webhooks for fpga and sgx.

Signed-off-by: Hyeongju Johannes Lee <[email protected]>
@@ -189,7 +189,7 @@ func describe() {
framework.Logf("found card and renderD from the log")
})

ginkgo.Context("When [Deployment:monitoring] deployment is applied [Resource:i915]", func() {
ginkgo.Context("When [Deployment:monitoring] deployment is applied", ginkgo.Label("Resource:i915"), func() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deployment:monitoring should also be converted into a label.

@@ -200,13 +200,13 @@ func describe() {
})
})

ginkgo.Context("When [Deployment:healthManagement] deployment is applied [Resource:i915]", func() {
ginkgo.Context("When [Deployment:healthManagement] deployment is applied", ginkgo.Label("Resource:i915"), func() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto for Deployment:healthManagement

ginkgo.It("check if i915 resources is available", func(ctx context.Context) {
createPluginAndVerifyExistence(f, ctx, healthMgmtPath, "gpu.intel.com/i915")
})
})

ginkgo.Context("When [Deployment:resourceManager] deployment is applied [Resource:i915]", func() {
ginkgo.Context("When [Deployment:resourceManager] deployment is applied", ginkgo.Label("Resource:i915"), func() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and for ditto for Deployment:resourceManager

Copy link
Contributor

@tkatila tkatila left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some suggestions or thoughts:

  • As we don't have to regex items from long strings anymore, we could drop prefixes like "Device:". The label is "fpga" or "qat", adding the "Device:" prefix doesn't add anything imo.
    ** Same could be done for the "App:" but I'm ok to keep it there.
  • Could we drop the "NoApp"? I'm not sure myself, but would like to entertain the idea. :)

Are you verifying that the tests stay the same with the changes? i.e. running with dry-run?

Also, sadly, using the labels doesn't make it much more readable. It's still quite bad.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve e2e labels matters
2 participants