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

BasicFastqWriterTest.test_flush_not_spammed fails due to additional flush() calls #688

Open
jfcr-genome opened this issue Jan 1, 2025 · 2 comments

Comments

@jfcr-genome
Copy link

Environment

Git commit ID: 09900c48d6d0e899a155cbc185f3d8e7c511bf05
Java version:

$ java --version
openjdk 17.0.9 2023-10-17 LTS  
OpenJDK Runtime Environment Corretto-17.0.9.8.1 (build 17.0.9+8-LTS)  
OpenJDK 64-Bit Server VM Corretto-17.0.9.8.1 (build 17.0.9+8-LTS, mixed mode, sharing)

Description

While attempting to build gridss using mvn clean package, the following test failed:

-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Picked up JAVA_TOOL_OPTIONS: -XX:+UseSerialGC -Xmx4g -Xms32m
Running htsjdk.samtools.fastq.BasicFastqWriterTest
Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.033 sec <<< FAILURE! - in htsjdk.samtools.fastq.BasicFastqWriterTest
test_flush_not_spammed(htsjdk.samtools.fastq.BasicFastqWriterTest)  Time elapsed: 0.032 sec  <<< FAILURE!
java.lang.AssertionError
	at htsjdk.samtools.fastq.BasicFastqWriterTest.test_flush_not_spammed(BasicFastqWriterTest.java:36)


Results :

Failed tests:
  BasicFastqWriterTest.test_flush_not_spammed:36

Tests run: 1, Failures: 1, Errors: 0, Skipped: 0

[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  7.627 s
[INFO] Finished at: 2024-12-29T18:20:11+09:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.19.1:test (default-test) on project gridss: There are test failures.
[ERROR]
[ERROR] Please refer to /home/ut1tn/local/gridss/target/surefire-reports for the individual test results.
[ERROR] -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException

Test details (/home/ut1tn/local/gridss/target/surefire-reports/htsjdk.samtools.fastq.BasicFastqWriterTest.txt)

-------------------------------------------------------------------------------
Test set: htsjdk.samtools.fastq.BasicFastqWriterTest
-------------------------------------------------------------------------------
Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.033 sec <<< FAILURE! - in htsjdk.samtools.fastq.BasicFastqWriterTest
test_flush_not_spammed(htsjdk.samtools.fastq.BasicFastqWriterTest)  Time elapsed: 0.032 sec  <<< FAILURE!
java.lang.AssertionError
	at htsjdk.samtools.fastq.BasicFastqWriterTest.test_flush_not_spammed(BasicFastqWriterTest.java:36)

Analysis

The issue appears to be caused by an additional flush() call triggered by the PrintStream.checkError() method. This leads to an extra invocation of flush() and causes the assertion loggedStream.flushCalled <= 2 to fail.

Relevant PrintStream source code:
https://github.com/openjdk/jdk/blob/ad54d8dd832b22485d7ac45958cc4c9bfd70fbd2/src/java.base/share/classes/java/io/PrintStream.java#L476-L484

Proposed Workaround

Updating the OpenJDK version to one where this behavior is adjusted may resolve the issue. Alternatively, the test itself could be modified to account for the extra flush() invocation by relaxing the assertion condition (e.g., allowing flushCalled <= 3).

Steps to Reproduce

Use the provided commit (09900c4).
Run the following command:

mvn clean package

or

mvn test -Dtest=BasicFastqWriterTest

Observe the failure in BasicFastqWriterTest.test_flush_not_spammed.

@d-cameron
Copy link
Member

Strange that it's failing now. Does OpenJDK no longer allow hacking changes into dependencies by creating your own copy of the class?

The fix got merged upstream (samtools/htsjdk#1497) so updating htsjdk would make it go away but GRIDSS is now over a major version behind so there's likely to be breaking changes if the htsjdk dependency version was changed.

@d-cameron
Copy link
Member

Was this issue AI assisted/created?

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

No branches or pull requests

2 participants