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

[3.4] tests: Avoid testing package root tests in e2e #15562

Merged
merged 1 commit into from
Mar 28, 2023

Conversation

serathius
Copy link
Member

@serathius serathius commented Mar 27, 2023

Changes invocation from go test -timeout 30m -v -cpu 1,2,4 '' -v --count 1 go.etcd.io/etcd/tests/e2e to go test -timeout 30m -v -cpu 1,2,4 -v --count 1 go.etcd.io/etcd/tests/e2e (removes ''). Those braces caused tests run in root package instead of e2e.

Logs from a failed run https://github.com/etcd-io/etcd/actions/runs/4533349980/jobs/7986037591:

=== RUN   TestMain
    main_test.go:32: skip launching etcd server when invoked via go test
--- SKIP: TestMain (0.00s)
=== RUN   TestMain
    main_test.go:32: skip launching etcd server when invoked via go test
--- SKIP: TestMain (0.00s)
=== RUN   TestMain
    main_test.go:32: skip launching etcd server when invoked via go test
--- SKIP: TestMain (0.00s)
PASS
ok  	go.etcd.io/etcd	0.020s
Error: Process completed with exit code 143.

cc @ahrtr @ptabor

@serathius serathius mentioned this pull request Mar 27, 2023
@fuweid
Copy link
Member

fuweid commented Mar 27, 2023

I didn't reproduce it in my local. 143 - 128 = 15 sigterm maybe from github action.
I run into this issue in etcd-io/bbolt#373 (comment). It is caused by high load and killed by github action node-agent.

REF: actions/runner-images#6709

@ahrtr
Copy link
Member

ahrtr commented Mar 28, 2023

curious why we do not see such error on the head of release-3.4?

Changes invocation from `go test -timeout 30m -v -cpu 1,2,4 '' -v
--count 1 go.etcd.io/etcd/tests/e2e` to `go test -timeout 30m -v -cpu 1,2,4 -v --count 1 go.etcd.io/etcd/tests/e2e` (removes '').
Those braces caused tests to also run in root package.

Signed-off-by: Marek Siarkowicz <siarkowicz@google.com>
@serathius serathius changed the title tests: Fix running e2e tests tests: Avoid testing package root tests in e2e Mar 28, 2023
@serathius
Copy link
Member Author

I think the problem is that running tests in root package results in test logs not being flushed properly. They are only flushed when tests finishes. So if the test is killed by GitHub as @fuweid mentioned, it causes logs to be missing.

On my local machine change from this PR results in logs being properly flushed, instead of flushed on the end.
@fuweid please confirm if it's the same for you.

@serathius
Copy link
Member Author

I still think it's worth merging this PR as without it, it's impossible to debug runs like https://github.com/etcd-io/etcd/actions/runs/4533349980/jobs/7986037591

Copy link
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM(non-binding)

Would you mind to update the pr title with prefix [3.4]? It's easy to review during release. Thanks!

@fuweid
Copy link
Member

fuweid commented Mar 28, 2023

please confirm if it's the same for you.

Yes. Thanks for the patch!

@serathius serathius changed the title tests: Avoid testing package root tests in e2e [3.4] tests: Avoid testing package root tests in e2e Mar 28, 2023
@serathius serathius merged commit 2b189d8 into etcd-io:release-3.4 Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants