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

JENKINS-54052 - Honor quietPeriod when scheduling multibranch builds #131

Closed
wants to merge 1 commit into from

Conversation

atanasenko
Copy link
Contributor

No description provided.

@abayer abayer self-requested a review November 5, 2018 19:57
@stephenc stephenc closed this Nov 16, 2018
@stephenc stephenc reopened this Nov 16, 2018
@stephenc
Copy link
Member

Test failures seem legit

@KostyaSha
Copy link
Member

Test failures seem legit

Do you mean this?

[WARNING] Flakes: 

[WARNING] integration.BrandingTest.given_orgFolderWithI18nRepos_when_indexing_then_repoNamesEncoded(integration.BrandingTest)

[ERROR]   Run 1: BrandingTest.cleanOutAllItems:97 class integration.harness.BasicMultiBranchProject is missing its descriptor

[INFO]   Run 2: PASS

@atanasenko atanasenko force-pushed the JENKINS-54052 branch 2 times, most recently from 9cb5bc5 to a42c034 Compare February 2, 2019 07:12
@bitwiseman bitwiseman closed this May 7, 2019
@bitwiseman bitwiseman reopened this May 7, 2019
@bitwiseman
Copy link
Contributor

@atanasenko Verified the test failures are real. Please take a look.

Copy link
Contributor

@bitwiseman bitwiseman left a comment

Choose a reason for hiding this comment

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

Test fixes needed.

@bitwiseman
Copy link
Contributor

@stephenc @KostyaSha
Why would all these tests start failing (and passing on rerun) due to this one change? I can't make sense of it.

@slide
Copy link
Member

slide commented Nov 18, 2019

I see failures on tests when I build master locally (on Windows), but CI is not seeing it. I am not sure what the issue is.

@slide
Copy link
Member

slide commented Dec 19, 2019

I believe the failures here are because the FreeStyleProject that is used in the tests does not have a quiet period set and when the quiet period is not set, Jenkins uses the default of 5 down in the class hierarchy somewhere. This seems to cause one of the tests to timeout, but it is not cleaned up correctly and so there are files in use that other tests try to clean up which can't be. I have worked around this by setting a quiet period of 0 for the FreeStyleProject used in the tests. This allows tests to pass. I will paste my fix after a meeting I have.

@atanasenko
Copy link
Contributor Author

@slide oh cool, thanks for finding that

@slide
Copy link
Member

slide commented Dec 19, 2019

I don't know if its a valid failure though, meaning, will the plugin not work in production in a similar way, someone with more knowledge would need to comment on that.

@bitwiseman
Copy link
Contributor

@slide
You didn't end up pasting in your fix.

@slide
Copy link
Member

slide commented Jan 30, 2020

You are very correct, I hope I didn't delete that repo! Let me look for it

@slide
Copy link
Member

slide commented Jan 30, 2020

diff --git a/src/test/java/integration/harness/BasicBranchProjectFactory.java b/src/test/java/integration/harness/BasicBranchProjectFactory.java
index b0f6658..bfc3ede 100644
--- a/src/test/java/integration/harness/BasicBranchProjectFactory.java
+++ b/src/test/java/integration/harness/BasicBranchProjectFactory.java
@@ -47,6 +47,10 @@ public class BasicBranchProjectFactory extends BranchProjectFactory<FreeStylePro
     public FreeStyleProject newInstance(Branch branch) {
         FreeStyleProject job = new FreeStyleProject(getOwner(), branch.getEncodedName());
         setBranch(job, branch);
+        try {
+            job.setQuietPeriod(0);
+        } catch(IOException e) {
+        }
         return job;
     }
 

@g-doc
Copy link

g-doc commented Apr 14, 2020

Hello, I would be very interested in getting this fix ! Any chance to have this PR merged ?

@bitwiseman
Copy link
Contributor

Closing in favor of #190

@bitwiseman bitwiseman closed this May 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants