Skip to content

Comments

HDDS-10081. Copy and migrate Hadoop base test classes to JUnit5#5939

Merged
adoroszlai merged 6 commits intoapache:masterfrom
adoroszlai:HDDS-10081
Jan 11, 2024
Merged

HDDS-10081. Copy and migrate Hadoop base test classes to JUnit5#5939
adoroszlai merged 6 commits intoapache:masterfrom
adoroszlai:HDDS-10081

Conversation

@adoroszlai
Copy link
Contributor

What changes were proposed in this pull request?

Ozone relies on base test classes from Hadoop for:

  • TestOzoneConfigurationFields
  • filesystem contract tests

Currently those base classes are implemented using JUnit4 (migration is in progress in Hadoop repo).

This PR:

  • copies the base classes from Hadoop 3.3.6
  • migrates them to JUnit5 as much as possible (incorporating in-progress changes from Hadoop) without touching the concrete implementation subclasses in Ozone

TestConfigurationFieldsBase is completely migrated.

Abstract contract tests:

  • assertions migrated to AssertJ (HADOOP-19025)
  • extends Assert removed from AbstractFSContractTestBase and ContractTestUtils
  • JUnit5 lifecycle annotations added (but JUnit4-ones also kept)
  • @Test annotations left unchanged (still using JUnit4)
  • slightly tweaked for checkstyle/findbugs (but still need some suppression), since rules in Hadoop seem more lenient

Most Ozone contract tests use Parameterized runner, which requires quite some refactoring for migrating to JUnit5. HDDS-10074 will finish that by refactoring Ozone's contract tests to JUnit5, and simply replacing @Test annotations in the abstract contract test base classes.

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

How was this patch tested?

Existing integration (contract) check in CI:

Tests run: 357, Failures: 0, Errors: 0, Skipped: 6

@adoroszlai adoroszlai added the test label Jan 7, 2024
@adoroszlai adoroszlai self-assigned this Jan 7, 2024
@SaketaChalamchala
Copy link
Contributor

@hemantk-12 can you please take a look?

@SaketaChalamchala
Copy link
Contributor

@swamirishi could you please take a look as well?

@adoroszlai
Copy link
Contributor Author

This might seem to be a large change, but most of the code is copied from Hadoop. Since this code is intended to be temporary in Ozone, I don't think we need to inspect it in detail, or adapt to Ozone coding conventions (beyond what checkstyle/findbugs complains about).

Having the same number of tests before/after the change, and with all tests passing, I think it should be fine.

If it helps during review, the follow-up change (conversion of Ozone contract tests to JUnit5) can be peeked at: adoroszlai@708f49b.

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 @adoroszlai for the patch.

Approving it without paying much attention to details as it is copied.

@adoroszlai
Copy link
Contributor Author

Thanks @hemantk-12 for the review. I pushed a new commit that replaces an import, because it is no longer accepted in Ozone after HDDS-10099:

-import org.apache.hadoop.thirdparty.com.google.common.base.Charsets;
+import com.google.common.base.Charsets;

Copy link
Contributor

@smengcl smengcl left a comment

Choose a reason for hiding this comment

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

lgtm at a glance. Thanks @adoroszlai

@adoroszlai adoroszlai merged commit 17be182 into apache:master Jan 11, 2024
@adoroszlai adoroszlai deleted the HDDS-10081 branch January 11, 2024 08:54
@adoroszlai
Copy link
Contributor Author

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

jojochuang pushed a commit to jojochuang/ozone that referenced this pull request Feb 28, 2024
jojochuang added a commit to jojochuang/ozone that referenced this pull request Mar 15, 2024
…t5 (apache#5939)"

This reverts commit 17be182.

 Conflicts:
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractAppendTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractConcatTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractContentSummaryTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractCopyFromLocalTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractCreateTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractDeleteTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractEtagTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractGetFileStatusTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractLeaseRecoveryTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractMkdirTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractMultipartUploaderTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractOpenTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractRenameTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractRootDirectoryTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractSafeModeTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractSeekTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractSetTimesTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractUnbufferTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractFSContractTestBase.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/tools/contract/AbstractContractDistCpTest.java

Change-Id: I7565c9d60e33f41b637a128fed33058bf429caae
jojochuang added a commit to jojochuang/ozone that referenced this pull request May 29, 2024
…t5 (apache#5939)"

This reverts commit 17be182.

 Conflicts:
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractAppendTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractConcatTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractContentSummaryTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractCopyFromLocalTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractCreateTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractDeleteTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractEtagTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractGetFileStatusTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractLeaseRecoveryTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractMkdirTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractMultipartUploaderTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractOpenTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractRenameTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractRootDirectoryTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractSafeModeTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractSeekTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractSetTimesTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractContractUnbufferTest.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/contract/AbstractFSContractTestBase.java
	hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/tools/contract/AbstractContractDistCpTest.java

Change-Id: I7565c9d60e33f41b637a128fed33058bf429caae
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.

4 participants