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 tests fail with go 1.13 #1329

Closed
chaodaiG opened this issue Sep 3, 2019 · 11 comments · Fixed by knative/serving#5398
Closed

E2E tests fail with go 1.13 #1329

chaodaiG opened this issue Sep 3, 2019 · 11 comments · Fixed by knative/serving#5398
Assignees

Comments

@chaodaiG
Copy link
Contributor

chaodaiG commented Sep 3, 2019

When running serving tests with go1.13, they would fail complaining flags provided but not defined.
For example:

# clone knative/serving
go test -v -tags=e2e -count=1 ./test/e2e -run ^TestAutoscaleUpDownUp$

Got errors:

flag provided but not defined: -test.v
@chaodaiG
Copy link
Contributor Author

chaodaiG commented Sep 3, 2019

/assign @steuhs

@srinivashegde86
Copy link
Contributor

probably related - golang/go#31859

@adrcunha adrcunha changed the title Test fail with go1.13 E2E tests fail with go 1.13 Sep 4, 2019
@adrcunha
Copy link
Contributor

adrcunha commented Sep 4, 2019

This seems to be caused due to the way flags are parsed in serving/test/e2e:

$ go test -v -count=1 ./pkg/... -run ^TestAutoscaleUpDownUp$
testing: warning: no tests to run
PASS
ok      knative.dev/serving/pkg/activator       0.294s [no tests to run]
?       knative.dev/serving/pkg/activator/config        [no test files]
testing: warning: no tests to run
...
$ go test -v -tags=e2e -count=1 ./test/e2e -run ^TestAutoscaleUpDownUpX$
flag provided but not defined: -test.v
Usage of /tmp/go-build413005213/b001/e2e.test:
...

@adrcunha
Copy link
Contributor

adrcunha commented Sep 4, 2019

We're running go 1.12 on Prow. The issue described happens with go 1.13rc1.

@adrcunha
Copy link
Contributor

adrcunha commented Sep 4, 2019

istio is facing the same issue: istio/istio#16797

@adrcunha
Copy link
Contributor

adrcunha commented Sep 4, 2019

Root cause: custom flag initialization function (https://github.com/knative/serving/blob/master/test/e2e_flags.go#L66) calling flag.Parse before the testing flags have been registered.

It seems to me that simply omitting flag.Parse() would work, as it will be called later by the testing package.

@howardjohn
Copy link

We fixed a similar issue in Istio by parsing in test main instead of init fwiw

@adrcunha
Copy link
Contributor

adrcunha commented Sep 4, 2019

Interesting, thanks John. @steuhs if we go that way, make sure test-infra is updated because otherwise using TestMain might break the presubmit build tests depending on what it does or expects.

@chaodaiG
Copy link
Contributor Author

chaodaiG commented Sep 4, 2019

@steuhs is OOO this week
/assign

@chaodaiG
Copy link
Contributor Author

chaodaiG commented Sep 4, 2019

/unassign @steuhs

@chaodaiG
Copy link
Contributor Author

chaodaiG commented Sep 5, 2019

In go1.13 testing level flags (i.e. -v) are registered in testing.Init() according to release note as part of generated main() function, which is executed later than global variable initialization, such as initializeServingFlags call, which failed to parse flags as they are not registered yet.

Cynocracy pushed a commit to Cynocracy/test-infra that referenced this issue Jun 13, 2020
* Add a library to parse config map values.

* Add AsString and comments.

* Add ParseFunc type.
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 a pull request may close this issue.

5 participants