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

tests/framework/integration: Fail BeforeTest nesting early #13807

Merged
merged 1 commit into from
Apr 7, 2022

Conversation

endocrimes
Copy link
Contributor

Currently there are a handful of tests within etcd that silently fail because LeakDetection will skip the test before it manages to hit this check.

Here we move the check to the beginning of the process to highlight these cases earlier, and to avoid them accidentally presenting as leaks.

@endocrimes
Copy link
Contributor Author

toomanyrequests: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: https://www.docker.com/increase-rate-limit

CI failure is unrelated, but this will fail until a couple of my other open PRs are merged. (Re-running the full suite locally now with this commit to find more cases before folks hit the magical run button here too)

@endocrimes
Copy link
Contributor Author

[tests(dani/before-test-fix)] $ cat test.out | grep  FAIL
=== FAIL: integration/clientv3 TestWatchCancelRunning (0.10s)
=== FAIL: integration/clientv3/experimental/recipes TestMutexTryLockSingleNode (0.08s)
=== FAIL: integration/clientv3/experimental/recipes TestMutexTryLockMultiNode (0.09s)

Looks like #13806 and #13804 should resolve all of the current integration cases of this particular issue 🎉

Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

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

LGTM

@serathius
Copy link
Member

@endocrimes feel free to ignore any semaphoreci test failures. On main branch we don't use it as a signal due to docker pull failures. We are planning to remove it #13448, however release-3.3 branch has no other tests.

Copy link
Member

@serathius serathius left a comment

Choose a reason for hiding this comment

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

Please rebase

Currently there are a handful of tests within etcd that silently fail
because LeakDetection will skip the test before it manages to hit this
check.

Here we move the check to the beginning of the process to highlight
these cases earlier, and to avoid them accidentally presenting as leaks.
@endocrimes endocrimes force-pushed the dani/before-test-fix branch from fa828b0 to 7cc00ec Compare April 7, 2022 13:11
@endocrimes
Copy link
Contributor Author

@serathius done

@serathius serathius merged commit a5b9f72 into etcd-io:main Apr 7, 2022
@endocrimes endocrimes deleted the dani/before-test-fix branch April 7, 2022 14:12
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