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 incorrect classpath leading to broken tests #739

Merged
merged 1 commit into from
May 14, 2024

Conversation

jtnord
Copy link
Member

@jtnord jtnord commented May 14, 2024

The classpath on windows was pure fantasy and worked by fluke only if java and all the code and dependencies where on the same drive.

if any code was on a different drive the classpath was mangled which leads to tests failing.

e.g. the classpath would end up being something like

D;\source\github\jenkins\remoting\target\test-classes;D;\source\github\jenkins\remoting\target\classes;C;\Users\jnord\.m2\repository\args4j\args4j\2.33\args4j-2.33.jar;C;\Users\jnord\.m2\repository\org\glassfish\tyrus\bundles\tyrus-standalone-client-jdk\2.1.5\tyrus-standalone-client-jdk-2.1.5.jar;C;\Users\jnord\.m2\repository\org\jenkins-ci\constant-pool-scanner\1.2\constant-pool-scanner-1.2.jar;C;\Users\jnord\.m2\repository\com\github\spotbugs\spotbugs-annotations\4.8.5\spotbugs-annotations-4.8.5.jar;C;\Users\jnord\.m2\repository\jakarta\websocket\jakarta.websocket-client-api\2.1.1\jakarta.websocket-client-api-2.1.1.jar;C;\Users\jnord\.m2\repository\net\jcip\jcip-annotations\1.0\jcip-annotations-1.0.jar;C;\Users\jnord\.m2\repository\org\kohsuke\access-modifier-annotation\1.33\access-modifier-annotation-1.33.jar;C;\Users\jnord\.m2\repository\org\jenkins-ci\annotation-indexer\1.17\annotation-indexer-1.17.jar;C;\Users\jnord\.m2\repository\com\google\guava\guava\31.0.1-jre\guava-31.0.1-jre.jar;C;\Users\jnord\.m2\repository\com\google\guava\failureaccess\1.0.1\failureaccess-1.0.1.jar;C;\Users\jnord\.m2\repository\com\google\guava\listenablefuture\9999.0-empty-to-avoid-conflict-with-guava\listenablefuture-9999.0-empty-to-avoid-conflict-with-guava.jar;C;\Users\jnord\.m2\repository\com\google\code\findbugs\jsr305\3.0.2\jsr305-3.0.2.jar;C;\Users\jnord\.m2\repository\org\checkerframework\checker-qual\3.12.0\checker-qual-3.12.0.jar;C;\Users\jnord\.m2\repository\com\google\errorprone\error_prone_annotations\2.7.1\error_prone_annotations-2.7.1.jar;C;\Users\jnord\.m2\repository\com\google\j2objc\j2objc-annotations\1.3\j2objc-annotations-1.3.jar;C;\Users\jnord\.m2\repository\commons-io\commons-io\2.16.1\commons-io-2.16.1.jar;C;\Users\jnord\.m2\repository\org\junit\jupiter\junit-jupiter\5.10.2\junit-jupiter-5.10.2.jar;C;\Users\jnord\.m2\repository\org\junit\jupiter\junit-jupiter-api\5.10.2\junit-jupiter-api-5.10.2.jar;C;\Users\jnord\.m2\repository\org\opentest4j\opentest4j\1.3.0\opentest4j-1.3.0.jar;C;\Users\jnord\.m2\repository\org\junit\platform\junit-platform-commons\1.10.2\junit-platform-commons-1.10.2.jar;C;\Users\jnord\.m2\repository\org\junit\jupiter\junit-jupiter-engine\5.10.2\junit-jupiter-engine-5.10.2.jar;C;\Users\jnord\.m2\repository\org\junit\jupiter\junit-jupiter-params\5.10.2\junit-jupiter-params-5.10.2.jar;C;\Users\jnord\.m2\repository\org\apiguardian\apiguardian-api\1.1.2\apiguardian-api-1.1.2.jar;C;\Users\jnord\.m2\repository\org\junit\vintage\junit-vintage-engine\5.10.2\junit-vintage-engine-5.10.2.jar;C;\Users\jnord\.m2\repository\org\junit\platform\junit-platform-engine\1.10.2\junit-platform-engine-1.10.2.jar;C;\Users\jnord\.m2\repository\junit\junit\4.13.2\junit-4.13.2.jar;C;\Users\jnord\.m2\repository\org\hamcrest\hamcrest-core\2.2\hamcrest-core-2.2.jar;C;\Users\jnord\.m2\repository\org\bouncycastle\bcpkix-jdk18on\1.78.1\bcpkix-jdk18on-1.78.1.jar;C;\Users\jnord\.m2\repository\org\bouncycastle\bcutil-jdk18on\1.78.1\bcutil-jdk18on-1.78.1.jar;C;\Users\jnord\.m2\repository\org\bouncycastle\bcprov-jdk18on\1.78.1\bcprov-jdk18on-1.78.1.jar;C;\Users\jnord\.m2\repository\org\hamcrest\hamcrest\2.2\hamcrest-2.2.jar;C;\Users\jnord\.m2\repository\org\jenkins-ci\test-annotations\1.4\test-annotations-1.4.jar;C;\Users\jnord\.m2\repository\org\mockito\mockito-core\5.11.0\mockito-core-5.11.0.jar;C;\Users\jnord\.m2\repository\net\bytebuddy\byte-buddy\1.14.12\byte-buddy-1.14.12.jar;C;\Users\jnord\.m2\repository\net\bytebuddy\byte-buddy-agent\1.14.12\byte-buddy-agent-1.14.12.jar;C;\Users\jnord\.m2\repository\org\objenesis\objenesis\3.3\objenesis-3.3.jar;C;\Users\jnord\.m2\repository\org\ow2\asm\asm\9.7\asm-9.7.jar;

which leads to
Error: Unable to initialize main class hudson.remoting.Launcher

This currently works in CI and elsewhere by pure fluke that the paths without drives
(\Users\jnord\.m2\repository\org\hamcrest\hamcrest\2.2\hamcrest-2.2.jar) use the current drive - which in CI will always be C: for java and the source code and maven, and the path C and D are silently ignored as invalid.

amends #495

Testing done

on windows:

ran mvn clean test -Dtest=PipeTest#testRemoteWrite before this change and it fails (as do so many other tests!)
ran mvn clean test -Dtest=PipeTest#testRemoteWrite after this change and validated the test passes

Linux testing will be performed by CI

Submitter checklist

Preview Give feedback

The classpath on windows was pure fantasy and worked by fluke only if
java and all the code and dependencies where on the same drive.

if any code was on a different drive the classpath was mangled which
leads to tests failing.

e.g.  the classpath would end up being something like

D;\source\github\jenkins\remoting\target\test-classes;D;\source\github\jenkins\remoting\target\classes;C;\Users\jnord\.m2\repository\args4j\args4j\2.33\args4j-2.33.jar;C;\Users\jnord\.m2\repository\org\glassfish\tyrus\bundles\tyrus-standalone-client-jdk\2.1.5\tyrus-standalone-client-jdk-2.1.5.jar;C;\Users\jnord\.m2\repository\org\jenkins-ci\constant-pool-scanner\1.2\constant-pool-scanner-1.2.jar;C;\Users\jnord\.m2\repository\com\github\spotbugs\spotbugs-annotations\4.8.5\spotbugs-annotations-4.8.5.jar;C;\Users\jnord\.m2\repository\jakarta\websocket\jakarta.websocket-client-api\2.1.1\jakarta.websocket-client-api-2.1.1.jar;C;\Users\jnord\.m2\repository\net\jcip\jcip-annotations\1.0\jcip-annotations-1.0.jar;C;\Users\jnord\.m2\repository\org\kohsuke\access-modifier-annotation\1.33\access-modifier-annotation-1.33.jar;C;\Users\jnord\.m2\repository\org\jenkins-ci\annotation-indexer\1.17\annotation-indexer-1.17.jar;C;\Users\jnord\.m2\repository\com\google\guava\guava\31.0.1-jre\guava-31.0.1-jre.jar;C;\Users\jnord\.m2\repository\com\google\guava\failureaccess\1.0.1\failureaccess-1.0.1.jar;C;\Users\jnord\.m2\repository\com\google\guava\listenablefuture\9999.0-empty-to-avoid-conflict-with-guava\listenablefuture-9999.0-empty-to-avoid-conflict-with-guava.jar;C;\Users\jnord\.m2\repository\com\google\code\findbugs\jsr305\3.0.2\jsr305-3.0.2.jar;C;\Users\jnord\.m2\repository\org\checkerframework\checker-qual\3.12.0\checker-qual-3.12.0.jar;C;\Users\jnord\.m2\repository\com\google\errorprone\error_prone_annotations\2.7.1\error_prone_annotations-2.7.1.jar;C;\Users\jnord\.m2\repository\com\google\j2objc\j2objc-annotations\1.3\j2objc-annotations-1.3.jar;C;\Users\jnord\.m2\repository\commons-io\commons-io\2.16.1\commons-io-2.16.1.jar;C;\Users\jnord\.m2\repository\org\junit\jupiter\junit-jupiter\5.10.2\junit-jupiter-5.10.2.jar;C;\Users\jnord\.m2\repository\org\junit\jupiter\junit-jupiter-api\5.10.2\junit-jupiter-api-5.10.2.jar;C;\Users\jnord\.m2\repository\org\opentest4j\opentest4j\1.3.0\opentest4j-1.3.0.jar;C;\Users\jnord\.m2\repository\org\junit\platform\junit-platform-commons\1.10.2\junit-platform-commons-1.10.2.jar;C;\Users\jnord\.m2\repository\org\junit\jupiter\junit-jupiter-engine\5.10.2\junit-jupiter-engine-5.10.2.jar;C;\Users\jnord\.m2\repository\org\junit\jupiter\junit-jupiter-params\5.10.2\junit-jupiter-params-5.10.2.jar;C;\Users\jnord\.m2\repository\org\apiguardian\apiguardian-api\1.1.2\apiguardian-api-1.1.2.jar;C;\Users\jnord\.m2\repository\org\junit\vintage\junit-vintage-engine\5.10.2\junit-vintage-engine-5.10.2.jar;C;\Users\jnord\.m2\repository\org\junit\platform\junit-platform-engine\1.10.2\junit-platform-engine-1.10.2.jar;C;\Users\jnord\.m2\repository\junit\junit\4.13.2\junit-4.13.2.jar;C;\Users\jnord\.m2\repository\org\hamcrest\hamcrest-core\2.2\hamcrest-core-2.2.jar;C;\Users\jnord\.m2\repository\org\bouncycastle\bcpkix-jdk18on\1.78.1\bcpkix-jdk18on-1.78.1.jar;C;\Users\jnord\.m2\repository\org\bouncycastle\bcutil-jdk18on\1.78.1\bcutil-jdk18on-1.78.1.jar;C;\Users\jnord\.m2\repository\org\bouncycastle\bcprov-jdk18on\1.78.1\bcprov-jdk18on-1.78.1.jar;C;\Users\jnord\.m2\repository\org\hamcrest\hamcrest\2.2\hamcrest-2.2.jar;C;\Users\jnord\.m2\repository\org\jenkins-ci\test-annotations\1.4\test-annotations-1.4.jar;C;\Users\jnord\.m2\repository\org\mockito\mockito-core\5.11.0\mockito-core-5.11.0.jar;C;\Users\jnord\.m2\repository\net\bytebuddy\byte-buddy\1.14.12\byte-buddy-1.14.12.jar;C;\Users\jnord\.m2\repository\net\bytebuddy\byte-buddy-agent\1.14.12\byte-buddy-agent-1.14.12.jar;C;\Users\jnord\.m2\repository\org\objenesis\objenesis\3.3\objenesis-3.3.jar;C;\Users\jnord\.m2\repository\org\ow2\asm\asm\9.7\asm-9.7.jar;

which leads to
Error: Unable to initialize main class hudson.remoting.Launcher

This currently works in CI and elsewhere by pure fluke that the paths
without drives
(\Users\jnord\.m2\repository\org\hamcrest\hamcrest\2.2\hamcrest-2.2.jar)
use the current drive - which in CI will always be C: for java and the
source code and maven

amends jenkinsci#495
@jtnord jtnord requested a review from a team May 14, 2024 11:11
@jglick jglick added the test label May 14, 2024
Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

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

any idea why this even existed before?

Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

0888b48 FTR

@jglick jglick merged commit 05d08a5 into jenkinsci:master May 14, 2024
13 checks passed
@jtnord
Copy link
Member Author

jtnord commented May 14, 2024

any idea why this even existed before?

the original code (prior to #495) used getClass().getClassLoader() which in some (ancient?) versions of maven was returned a URL classloader. (file:// URLs).
this may also be due to the use of a shortened URL in surefire-booter for long classpaths, but as shows this passes on linux and windows in CI (and in an IDE).

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

Successfully merging this pull request may close these issues.

3 participants