Skip to content

Conversation

@wzhallright
Copy link
Contributor

@wzhallright wzhallright commented Dec 21, 2023

What changes were proposed in this pull request?

Improve assertTrue assertions in hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om
Assertions in the form:

assertTrue(<string>.contains(...))

do not provide any information about the actual value, if the assertion fails.

AssertionFailedError: expected: <true> but was: <false>

This can be improved by replacing them with:

import static org.assertj.core.api.Assertions.assertThat;
assertThat(<string>).contains(...)

which gives us more info in the form:

AssertionError: 

Expecting:
 <"actual string">
to contain:
 <"part"> 

Similarly, assertions about inequality relations, e.g.:

assertTrue(<x> > <y>)

can be replaced with:

assertThat(<x>).isGreaterThan(<y>);

The goal of this task is to find and replace such assertTrue assertions.

What is the link to the Apache JIRA

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

CI

https://github.com/wzhallright/ozone/actions/runs/7295265484

@adoroszlai
Copy link
Contributor

@wzhallright please resolve conflicts

# Conflicts:
#	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestLeaderChoosePolicy.java
#	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java
#	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmBlockVersioning.java
@wzhallright
Copy link
Contributor Author

@wzhallright please resolve conflicts

Has resolved conflicts. Thanks for review.

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 @wzhallright for updating the patch. Let's wait with this a bit for other conflicting changes.

@hemantk-12
Copy link
Contributor

@wzhallright
Copy link
Contributor Author

@wzhallright, I think you have to wait for HDDS-9922 and update https://github.com/apache/ozone/pull/5838/files?diff=unified&w=0#diff-7204a4e31ff9ef05e7839a26737265513d5aedc6ddeac72a9cbda7f263a0ddffR1826 as well.

OK,Thanks for review @hemantk-12. I will rebase and update it after #5838 is merged.

@adoroszlai
Copy link
Contributor

No need to wait/rebase, filesystem integration tests will be covered by separate sub-task later.

@adoroszlai adoroszlai merged commit 0bb733e into apache:master Dec 22, 2023
@adoroszlai
Copy link
Contributor

Thanks @wzhallright for the improvement.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants