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.

Sign up for GitHub

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

Jump to bottom

test.EnsureTearDown does not cleanup resources completely #8560

Closed
nak3 opened this issue Jul 6, 2020 · 3 comments · Fixed by #8566
Closed

test.EnsureTearDown does not cleanup resources completely #8560

nak3 opened this issue Jul 6, 2020 · 3 comments · Fixed by #8566
Assignees
@vagababov
Labels
area/test-and-release It flags unit/e2e/conformance/perf test issues for product features kind/bug Categorizes issue or PR as related to a bug.

Comments

@nak3
Copy link
Contributor

nak3 commented Jul 6, 2020 •
edited
Loading

In what area(s)?

/area test-and-release

What version of Knative?

Build on e9c1ba1

Actual Behavior

After EnsureCleanup() was introduced, some tests like TestServiceGenerateName, TestRouteAndConfigGenerateName, TestImagePullError, etc, do not clean up resources.
The reason is that, for example TestServiceGenerateName, test.EnsureTearDown registers empty names and names.Service is updated later.

Expected Behavior

  • EnsureCleanup() should cleanup resources completely.
  • Perhaps we should change to pass EnsureCleanup() names with its address like:
  test.EnsureTearDown(t, clients, &names)

Steps to Reproduce the Problem

Run TestServiceGenerateName and check ksvc remains after the test.

$ go test -v  -count=1  -tags=e2e  ./test/conformance/api/v1alpha1/... -run TestServiceGenerateName
@nak3 nak3 added the kind/bug Categorizes issue or PR as related to a bug. label Jul 6, 2020
@knative-prow-robot knative-prow-robot added the area/test-and-release It flags unit/e2e/conformance/perf test issues for product features label Jul 6, 2020
@nak3
Copy link
Contributor Author

nak3 commented Jul 6, 2020

/cc @julz @vagababov

@vagababov
Copy link
Contributor

/assign
I'll poke tomorrow

@vagababov
Copy link
Contributor

Yeah, I went with &names everywhere.

vagababov added a commit to vagababov/serving that referenced this issue Jul 6, 2020
@vagababov
Change cleanup func to take pointer to names
89f5fe4 
if the names change, it will still cleanup the proper resource.
Fixes knative#8560
@vagababov vagababov mentioned this issue Jul 6, 2020
Merged
knative-prow-robot pushed a commit that referenced this issue Jul 7, 2020
@vagababov
Change cleanup func to take pointer to names (#8566)
d74ecbe 
* Change cleanup func to take pointer to names

if the names change, it will still cleanup the proper resource.
Fixes #8560

* also perf tests

* :upgrade:

* one more
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@vagababov vagababov

Labels
area/test-and-release It flags unit/e2e/conformance/perf test issues for product features kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Change cleanup func to take pointer to names vagababov/serving
3 participants
@vagababov @nak3 @knative-prow-robot