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

Fix test flake preventing BOM release #394

Merged
merged 1 commit into from
Apr 30, 2024
Merged

Fix test flake preventing BOM release #394

merged 1 commit into from
Apr 30, 2024

Conversation

basil
Copy link
Member

@basil basil commented Apr 30, 2024

This test was flaky, as it passed locally and passed in jenkinsci/bom#3137 but has failed twice on the main branch (e.g. https://ci.jenkins.io/job/Tools/job/bom/job/master/2712/testReport/) with

java.lang.AssertionError: 

Expected: is "Get contextual object from internal APIs"
     but: was "This stage will be skipped."
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:6)
	at io.jenkins.plugins.pipelinegraphview.utils.PipelineStepApiLegacyTest.getAllStepsReturnsStepsForComplexParallelBranches(PipelineStepApiLegacyTest.java:217)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at org.jvnet.hudson.test.JenkinsRule$1.evaluate(JenkinsRule.java:656)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
	at java.base/java.lang.Thread.run(Thread.java:1583)

Looking into this further, it seems that PCT builds on the main branch have defined BRANCH_NAME=master as an environment variable and this causes the test to fail. I could reproduce this locally with export BRANCH_NAME=master. The test seemed to be trying to get a stage to be skipped, but when running on master the stage wasn't skipped and the test started failing. By changing the test to not rely on master I could get the test to pass regardless of what that environment variable was defined to.

Testing done

As described above

@basil
Copy link
Member Author

basil commented Apr 30, 2024

If perchance anyone happens to be awake in the next few hours and interested in helping, it would be great to take advantage of time zones to merge and release the above and restart a build of https://ci.jenkins.io/job/Tools/job/bom/job/master/ which will take several hours.

@basil
Copy link
Member Author

basil commented Apr 30, 2024

Specifically the test was relying on the branch being named something other than master, which was true for https://github.com/jenkinsci/pipeline-graph-view-plugin where the branch is named main and was also true for jenkinsci/bom#3137 where the branch was named PR-something-or-other, but not true for https://github.com/jenkinsci/bom where the branch is actually named master

Copy link
Member

@alecharp alecharp left a comment

Choose a reason for hiding this comment

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

This looks simple enough and the tests are passing.

@alecharp
Copy link
Member

@gabrieleara do you think you could approve and merge this pull request so it can unblock the release process of the bom? Thanks.

@timja timja merged commit c9e11fe into jenkinsci:main Apr 30, 2024
14 checks passed
@basil basil deleted the flake branch April 30, 2024 17:30
@basil basil mentioned this pull request Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants