Skip to content

HDDS-10058. Replace Hamcrest assertions#5919

Merged
adoroszlai merged 1 commit intoapache:masterfrom
adoroszlai:HDDS-10058
Jan 5, 2024
Merged

HDDS-10058. Replace Hamcrest assertions#5919
adoroszlai merged 1 commit intoapache:masterfrom
adoroszlai:HDDS-10058

Conversation

@adoroszlai
Copy link
Contributor

What changes were proposed in this pull request?

Replace Hamcrest assertThat and matchers with JUnit or AssertJ assertions, to reduce the number of test dependencies a bit.

https://issues.apache.org/jira/browse/HDDS-10058

How was this patch tested?

CI:
https://github.com/adoroszlai/ozone/actions/runs/7409626572

@adoroszlai adoroszlai added test dependencies Pull requests that update a dependency file labels Jan 4, 2024
@adoroszlai adoroszlai self-assigned this Jan 4, 2024
Copy link
Contributor

@hemantk-12 hemantk-12 left a comment

Choose a reason for hiding this comment

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

Thanks for the patch @adoroszlai .

LGTM+1.

<dependency>
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest-all</artifactId>
<artifactId>hamcrest-core</artifactId>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we try to get rid of core as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, let me try that.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see #5923

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we can remove hamcrest-core, but only because we also have hamcrest as a transitive dependency.

@adoroszlai adoroszlai merged commit 64e6bbd into apache:master Jan 5, 2024
@adoroszlai adoroszlai deleted the HDDS-10058 branch January 5, 2024 09:58
@adoroszlai
Copy link
Contributor Author

Thanks @hemantk-12, @kerneltime for the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants