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

RATIS-2169. mvnw test: Corrupted STDOUT by directly writing to native stream in forked JVM 1 #1162

Merged
merged 2 commits into from
Oct 7, 2024

Conversation

szetszwo
Copy link
Contributor

@szetszwo szetszwo commented Oct 7, 2024

@szetszwo
Copy link
Contributor Author

szetszwo commented Oct 7, 2024

The failed test is a known problem but not related this; see RATIS-2151.

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks @szetszwo for the patch.

Did it happen only locally or in some PR? I don't see such issue in logs collected from runs on master.

pom.xml Outdated
@@ -176,7 +176,7 @@
<maven-checkstyle-plugin.version>3.3.0</maven-checkstyle-plugin.version>
<maven-clover2-plugin.version>4.0.6</maven-clover2-plugin.version>
<maven-pdf-plugin.version>1.6.1</maven-pdf-plugin.version>
<maven-surefire-plugin.version>3.0.0-M4</maven-surefire-plugin.version>
<maven-surefire-plugin.version>3.5.1</maven-surefire-plugin.version>
Copy link
Contributor

Choose a reason for hiding this comment

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

Versions newer than 3.0.0-M4 are affected by https://issues.apache.org/jira/browse/SUREFIRE-1722

Maybe we have to decide which bug to live with ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good to know. Let's remain in 3.0.0-M4 then.

<junit.jupiter.version>5.10.1</junit.jupiter.version>
<jacoco.version>0.8.11</jacoco.version>
<junit-bom.version>5.11.2</junit-bom.version>
<jacoco.version>0.8.12</jacoco.version>
Copy link
Contributor

Choose a reason for hiding this comment

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

According to the comments on stack overflow:

upgrading jacoco-plugin from 0.8.1 to 0.8.4 did the job

But we already have 0.8.11, which is newer than both, so I guess this won't fix the problem.

We can upgrade anyway, Ozone has been using 0.8.12 for 6 months now (HDDS-10660).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am also aware that. Just wonder they might have a better fix in a newer version.

@szetszwo
Copy link
Contributor Author

szetszwo commented Oct 7, 2024

@adoroszlai , thanks for reviewing this!

Tested this change in #1156 . It was able to pass everything (with a few retries). Since this is just a build change, I would ignore the known test failures and merge this.

@szetszwo szetszwo merged commit da3f90b into apache:master Oct 7, 2024
9 of 12 checks passed
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

Successfully merging this pull request may close these issues.

2 participants