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

Try to avoid test hangs #3055

Conversation

jglick
Copy link
Member

@jglick jglick commented Sep 29, 2017

Observed a couple CI builds of older core branches hanging in this test for hours, I suspect because the test was failing to find the expected task but it was doing so in an endless loop that could not respond to Thread.interrupt despite the standard 3m timeout in JenkinsRule.

Possible that jenkinsci/jenkins-test-harness#67 (integrated in #2959) would have helped.

@reviewbybees

@jglick jglick requested a review from varmenise September 29, 2017 18:32
@ghost
Copy link

ghost commented Sep 29, 2017

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

@Jimilian
Copy link
Contributor

Jimilian commented Sep 29, 2017

Maybe it will be better to directly call maintain method of Queue two times? If everything is correct it should move the task to buildable, no?

@jglick
Copy link
Member Author

jglick commented Sep 30, 2017

Maybe. @varmenise’s code; I am not too interested in editing it heavily, I just want it to stop causing CI outages.

@jglick
Copy link
Member Author

jglick commented Sep 30, 2017

CI failure looks random:

Connect to repo.azure.jenkins.io:443 [repo.azure.jenkins.io/13.68.19.38] failed: Connection refused

@daniel-beck
Copy link
Member

Windows build passed, but I triggered it again anyway.

@jglick
Copy link
Member Author

jglick commented Oct 4, 2017

I am not sure what this

junit.framework.AssertionFailedError: test failure
	at junit.framework.Assert.fail(Assert.java:47)
	at org.jvnet.hudson.test.BuildTriggerTest.testSomething(BuildTriggerTest.java:7)

is about. There is no such test class or test method that I can see, in this repo or anywhere else. I do not have workspace view permission even if the workspace still exists.

@jglick
Copy link
Member Author

jglick commented Oct 5, 2017

AgentProtocolTest.testShouldNotDisableProtocolsForMigratedInstances failure, unrelated (#3012).

@jglick jglick requested a review from oleg-nenashev October 5, 2017 14:54
Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

🐝 whatever works

@oleg-nenashev oleg-nenashev added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Oct 8, 2017
@jglick jglick merged commit f81af78 into jenkinsci:master Oct 9, 2017
@jglick jglick deleted the QueueTest.shouldBeAbleToBlockFlyweightTaskAtTheLastMinute branch October 9, 2017 13:26
@jglick
Copy link
Member Author

jglick commented Oct 18, 2017

There is no such test class or test method that I can see, in this repo or anywhere else.

FTR this was src/test/java/org/jvnet/hudson/test/BuildTriggerTest.java in test/src/test/resources/hudson/tasks/maven-test-failure.zip. How it “escaped” to the project test results, I am not sure…

@jglick
Copy link
Member Author

jglick commented Oct 18, 2017

Ah, perhaps because we are archiving **/target/surefire-reports/*.xml when we should be doing only */target/surefire-reports/*.xml.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants