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

reuseForks=false #139

Merged
merged 1 commit into from
Apr 3, 2020
Merged

reuseForks=false #139

merged 1 commit into from
Apr 3, 2020

Conversation

jglick
Copy link
Member

@jglick jglick commented Apr 3, 2020

Trying to fix #135 (comment) by working around a regression from junit-team/junit4#1517.

@jglick jglick added the chore label Apr 3, 2020
@jglick jglick merged commit 080ce0e into jenkinsci:master Apr 3, 2020
@jglick jglick deleted the no-reuse-forks branch April 3, 2020 01:33
@jglick
Copy link
Member Author

jglick commented Apr 3, 2020

FTR there were several things going on, detecting from PCT failures.

First, there was a linkage error from ssh-slaves, which #135 corrects.

This was also triggering a Docker fixture error, corrected in jenkinsci/docker-fixtures#26. Initially was thought to be specific to DooD, but in fact turned out to be reproducible even when Surefire was running on the host.

But then #135 picked up junit-team/junit4#1517 which introduced an error visible only in certain modes, not in individual tests and apparently also not when you run tests in parallel under certain conditions. Seen for example in

V=2.150.3
mvn dependency:get -Dartifact=org.jenkins-ci.main:jenkins-war:$V:war
docker run -ti --rm -v /var/run/docker.sock:/var/run/docker.sock -v $HOME/.m2:/root/.m2 -v /tmp/out:/pct/out -v $HOME/.m2/repository/org/jenkins-ci/main/jenkins-war/$V/jenkins-war-$V.war:/pct/jenkins.war:ro -v $(pwd):/pct/plugin-src -e ARTIFACT_ID=mercurial -e SHARED_DOCKER_SERVICE=true jenkins/pct -mavenProperties maven.test.redirectTestOutputToFile=true:test=InjectedTest,CacheTest,CachingCustomDirSCMTest,CachingSCMTest

or more simply in

mvn test -Dmaven.test.redirectTestOutputToFile=true -Dtest=InjectedTest,CacheTest,CachingCustomDirSCMTest,CachingSCMTest

The JUnit regression was tracked down by bisecting after

diff --git pom.xml pom.xml
index 60f37f8..060942d 100644
--- pom.xml
+++ pom.xml
@@ -18,6 +18,11 @@
       <jenkins.version>2.150.3</jenkins.version>
       <java.level>8</java.level>
       <no-test-jar>false</no-test-jar>
+      <junit.version>4.13-SNAPSHOT</junit.version>
+      <test>InjectedTest,CacheTest,CachingCustomDirSCMTest,ChangelogTest#spacesInPaths[2]</test>
+      <enforcer.skip>true</enforcer.skip>
+      <maven.test.redirectTestOutputToFile>false</maven.test.redirectTestOutputToFile>
+      <surefire.rerunFailingTestsCount>1</surefire.rerunFailingTestsCount>
     </properties>
     
     <developers>
@@ -80,6 +85,11 @@
                 <scope>import</scope>
                 <type>pom</type>
             </dependency>
+            <dependency>
+                <groupId>junit</groupId>
+                <artifactId>junit</artifactId>
+                <version>${junit.version}</version>
+            </dependency>
         </dependencies>
     </dependencyManagement>
     <dependencies>
@@ -169,7 +179,7 @@
         <dependency>
             <groupId>org.jenkins-ci.test</groupId>
             <artifactId>docker-fixtures</artifactId>
-            <version>1.8</version>
+            <version>1.10-SNAPSHOT</version>
             <scope>test</scope>
         </dependency>
         <dependency>

@jglick
Copy link
Member Author

jglick commented Jan 4, 2021

Supposedly junit-team/junit4#1687 (and junit-team/junit4#1691?) planned for JUnit 1.13.2 will render this unnecessary. Still, it seems safest to keep reuseForks=false generally for this plugin. The overhead of a new JVM & class loading is significant even compared to the overhead of JenkinsRule, but these tests are quite slow even for JenkinsRule.

@jglick
Copy link
Member Author

jglick commented Feb 15, 2021

Acc. to junit-team/junit4#1652 (comment) updating to JUnit 4.13.2 would allow this to be reverted, if desired. Not sure it matters much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
1 participant