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

Add a 3 second delay to decrease failures in the threaded submodule test #1122

Conversation

ckpattar
Copy link
Contributor

@ckpattar ckpattar commented Apr 1, 2024

BEE-47337

The test case is failing at the assertion and clearly its the problem at Threads, where in the submoduleUpdate has not been completed yet and its proceeding to the assertions, adding a slight delay will make sure the submoduleupdate is complete.

Checklist

  • I have read the CONTRIBUTING doc
  • I have referenced the Jira issue related to my changes in one or more commit messages
  • Unit tests pass locally with my changes

Types of changes

What types of changes does your code introduce?

  • Test change (non-breaking change which updates dependencies or improves infrastructure)

@ckpattar ckpattar requested a review from a team as a code owner April 1, 2024 04:28
@github-actions github-actions bot added the tests Automated test addition or improvement label Apr 1, 2024
@MarkEWaite MarkEWaite changed the title [BEE-47337] added a delay of 3 seconds Add a 3 second delay to decrease failures in the threaded submodule test Apr 1, 2024
Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

Thanks!

@MarkEWaite MarkEWaite merged commit fb9d2ca into jenkinsci:master Apr 1, 2024
15 checks passed
@@ -77,7 +77,7 @@ public void testSubmoduleUpdateWithThreads() throws Exception {
.execute();
w.git.submoduleInit();
w.git.submoduleUpdate().threads(3).execute();

Thread.sleep(3000);
Copy link
Member

@jtnord jtnord Jun 18, 2024

Choose a reason for hiding this comment

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

given execute() does not return a Future and GitCommands can take a timeout, shouldn't this be method be considered synchronous?
As such wouldn't this be a production issue and not a test issue?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not willing at this point to spend the time to investigate if the method is synchronous. I had never seen this test fail in my tests on ci.jenkins.io or in my home lab, but it was a harmless addition, so I accepted it. Will propose a revert to tidy the test a little.

Copy link
Contributor

Choose a reason for hiding this comment

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

MarkEWaite added a commit to MarkEWaite/git-client-plugin that referenced this pull request Jun 18, 2024
jenkinsci#1122 (comment)
notes that the sleep does not actually help the test other than to delay
its completion.

This reverts commit fb9d2ca.
MarkEWaite added a commit that referenced this pull request Jun 18, 2024
#1122 (comment)
notes that the sleep does not actually help the test other than to delay
its completion.

This reverts commit fb9d2ca.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Automated test addition or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants