test(storage): exclude jqwik tests on Java 8#13846
Conversation
There was a problem hiding this comment.
Code Review
This pull request modifies the Maven configuration to conditionally load the jqwik dependency on JDK 11+ and exclude specific test files when compiling on Java 8. The review feedback points out that hardcoding individual test files in the exclusion list is fragile and risks losing standard JUnit test coverage on Java 8. It is recommended to separate jqwik property tests into dedicated files or packages to simplify the POM exclusions to wildcard patterns.
| <exclude>**/StorageV2ProtoUtilsTest.java</exclude> | ||
| <exclude>**/JsonUtilsTest.java</exclude> | ||
| <exclude>**/ChunkSegmenterTest.java</exclude> | ||
| <exclude>**/DefaultBufferedWritableByteChannelTest.java</exclude> | ||
| <exclude>**/MinFlushBufferedWritableByteChannelTest.java</exclude> | ||
| <exclude>**/DefaultBufferedReadableByteChannelTest.java</exclude> | ||
| <exclude>**/Crc32cValueTest.java</exclude> | ||
| <exclude>**/ITSyncingFileChannelTest.java</exclude> | ||
| <exclude>**/RewindableByteBufferContentTest.java</exclude> |
There was a problem hiding this comment.
Hardcoding individual test files in the POM's exclusion list is fragile and difficult to maintain. Additionally, if these files contain standard JUnit tests alongside jqwik property tests, excluding the entire file from compilation on Java 8 means we completely lose standard test coverage on Java 8.
Recommendation
- Separate Test Concerns: Move jqwik property tests into dedicated files ending with
PropertyTest.java(e.g.,JsonUtilsPropertyTest.java) or place them under ajqwikpackage. - Retain Java 8 Coverage: Keep standard JUnit tests in their original files (e.g.,
JsonUtilsTest.java) without any jqwik imports so they can compile and run on Java 8. - Simplify POM: This allows you to simplify the exclusions here to just the wildcard patterns (
**/*PropertyTest.javaand**/jqwik/**), removing the need to list individual files.
Problem
Jqwik version 1.9+ requires JDK 11+ at runtime. Since
google-cloud-javaruns tests on JDK 8 matrix environments, Jqwik's reflection-based class scanning fails when JSpecify 1.0.0 annotations are present, causingUnsupportedClassVersionErroror reflection scanner crashes during JUnit test discovery on Java 8.Solution
This PR introduces a clean profile-based exclusion to isolate Jqwik on Java 8 without impacting modern Java environments:
jqwik-testsprofile: Thenet.jqwik:jqwiklibrary is no longer loaded globally; it is only added to the test classpath on JDK 11+ ([11,)).exclude-jqwik-on-java8profile: Active only when the JDK is exactly1.8.testExcludesto skip compiling all Jqwik-based property tests (e.g.,*PropertyTest.java,jqwikpackages, etc.) to prevent compile errors when Jqwik is not on the classpath.RewindableByteBufferContentTest.javaon Java 8 as well, since it has an import reference to a nested class insideRewindableContentPropertyTest.javaand would otherwise pull it into compiler scope.Rationale