Skip to content

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.

@szetszwo szetszwo requested a review from adoroszlai October 7, 2024 05:34
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
<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
szetszwo added a commit to szetszwo/ratis that referenced this pull request Mar 30, 2025
szetszwo added a commit to szetszwo/ratis that referenced this pull request May 23, 2025
szetszwo added a commit to szetszwo/ratis that referenced this pull request May 23, 2025
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