-
-
Notifications
You must be signed in to change notification settings - Fork 9.3k
[Fixes JENKINS-75456] De-flake DisconnectNodeCommandTest #10425
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
[Fixes JENKINS-75456] De-flake DisconnectNodeCommandTest #10425
Conversation
|
I wasn't able to add the following labels: skip-changelog tests Check that the label exists and is spelt right then try again. |
2 similar comments
|
I wasn't able to add the following labels: skip-changelog tests Check that the label exists and is spelt right then try again. |
|
I wasn't able to add the following labels: skip-changelog tests Check that the label exists and is spelt right then try again. |
| } | ||
|
|
||
| @Test | ||
| public void disconnectNodeManyShouldSucceedEvenANodeIsSpecifiedTwice() throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Flaky test noted in PR description
| } | ||
|
|
||
| @Test | ||
| public void disconnectNodeShouldSucceedWithCause() throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Flaky test noted in PR description
| } | ||
|
|
||
| @Test | ||
| public void disconnectNodeManyShouldSucceed() throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Flaky test noted in PR description
| } | ||
|
|
||
| @Test | ||
| public void disconnectNodeShouldSucceed() throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Flakiness found in dependabot's PR build ref
| } | ||
|
|
||
| @Test | ||
| public void disconnectNodeManyShouldFailIfANodeDoesNotExist() throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No flakes seen yet, but since the code is similar to other flaky tests, I'm patching it by reusing assertOffline.
jglick
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Co-authored-by: Jesse Glick <[email protected]>
A1exKH
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gbhat618 LGTM.
MarkEWaite
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
|
This PR is now ready for merge. Merging it immediately to reduce flaky tests on the master branch /label ready-for-merge |
|
Congratulations on getting your very first Jenkins core pull request merged 🎉🥳 |
See JENKINS-75456, and taking reference from #10307
I've recently noticed flakes in the DisconnectNodeCommandTest, which have impacted the CI execution for my other PR: #10396.
The table below contains links to the test failures. The build numbers in the table correspond to the builds of my mentioned PR.
They always fail with
I can reproduce the failures on my laptop about 40% of the time. The failing test is random among the ones mentioned.
I ran the tests more than 25 times using the following command:
Example logs from local
local failure - disconnectNodeManyShouldSucceed
local failure - disconnectNodeManyShouldSucceedEvenANodeIsSpecifiedTwice
Testing done
Manual testing to reproduce the issue (see the details above). After the fix the tests are passing
Proposed changelog entries
Proposed changelog category
/label skip-changelog
/label tests
Proposed upgrade guidelines
N/A
Submitter checklist
@Restrictedor have@since TODOJavadocs, as appropriate.@Deprecated(since = "TODO")or@Deprecated(forRemoval = true, since = "TODO"), if applicable.evalto ease future introduction of Content Security Policy (CSP) directives (see documentation).Desired reviewers
@jglick, @Vlatombe
Before the changes are marked as
ready-for-merge:Maintainer checklist
upgrade-guide-neededlabel is set and there is a Proposed upgrade guidelines section in the pull request title (see example).lts-candidateto be considered (see query).