HDDS-10165. Replace GenericTestUtils temp dirs with `@TempDir` in ozone unit tests #6142

raju-balpande · 2024-02-01T06:04:04Z

What changes were proposed in this pull request?

Replace usage of GenericTestUtils test/temp dir methods with @TempDir in hadoop-ozone except in integration tests.

(see parent task HDDS-9103 for details)

The change covered..
Introduced local @TempDir variable in some cases.
Introduced global @TempDir at one location because the directory was shared among directories.
Removed the deletion logic of manually created temp directories.
Removed setup methods if it has nothing else then initializing the temp dir.

What is the link to the Apache JIRA

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

How was this patch tested?

Tested in CI-CI pipeline and a few manually.

adoroszlai · 2024-02-01T08:03:23Z

Thanks @raju-balpande for working on this.

There are some conflicts, please merge from master.

Also, one unit test failure looks related:

Error:  Tests run: 6, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 1.059 s <<< FAILURE! - in org.apache.hadoop.ozone.security.TestOzoneTokenIdentifier
Error:  org.apache.hadoop.ozone.security.TestOzoneTokenIdentifier.testReadWriteInProtobuf(Path)  Time elapsed: 0.064 s  <<< ERROR!
java.io.FileNotFoundException: /tokenFile (Permission denied)
	at java.io.FileOutputStream.open0(Native Method)
	at java.io.FileOutputStream.open(FileOutputStream.java:270)
	at java.io.FileOutputStream.<init>(FileOutputStream.java:213)
	at java.io.FileOutputStream.<init>(FileOutputStream.java:162)
	at org.apache.hadoop.ozone.security.TestOzoneTokenIdentifier.testReadWriteInProtobuf(TestOzoneTokenIdentifier.java:255)

(Please use code formatting (enclose in `) for @TempDir and other annotations. @... is used on GitHub to "mention" others (like @raju-balpande) and we don't want to bother random folks.)

adoroszlai · 2024-02-01T08:12:57Z

hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java

+  public void init(@TempDir Path pathTestDir) throws Exception {
    OzoneConfiguration configuration = new OzoneConfiguration();
-    testDir = GenericTestUtils.getRandomizedTestDir();
+    testDir = pathTestDir.toFile();


testDir can be annotated directly, then we don't need the parameter and the assignment.

Done. thanks.

adoroszlai · 2024-02-01T08:13:19Z

hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java

  private KeyManagerImpl keyManager;

  private Instant startDate;
-  private File testDir;


We can keep and annotate it, instead of adding parameter.

It is not used anywhere else hence added as a parameter instead of keeping as instance variable. It is also removed from the method cleanup.

adoroszlai · 2024-02-01T08:29:11Z

...ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerHttpServer.java

  @BeforeAll public static void setUp() throws Exception {
-    File base = new File(BASEDIR);


Here we can minimize change by adding @TempDir File base as parameter, instead of adding a member variable.

Done. thanks.

adoroszlai · 2024-02-01T08:32:49Z

...e/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestOzoneTokenIdentifier.java

+  public void testReadWriteInProtobuf(@TempDir Path baseDir) throws IOException {
    OzoneTokenIdentifier id = getIdentifierInst();
-    File idFile = new File(BASEDIR + "/tokenFile");
+    File idFile = baseDir.resolve("/tokenFile").toFile();


I think test failure is caused by /tokenFile being resolved as absolute path, not a child of baseDir.

Suggested change

File idFile = baseDir.resolve("/tokenFile").toFile();

File idFile = baseDir.resolve("tokenFile").toFile();

Done. thanks.

adoroszlai · 2024-02-01T12:59:03Z

hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java
 210: 'testDir' hides a field.
 361: 'testDir' hides a field.
 452: 'testDir' hides a field.
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestVolumeOwner.java
 39: Unused import - org.apache.hadoop.security.authentication.client.AuthenticationException.
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestParentAcl.java
 46: Unused import - org.apache.hadoop.security.authentication.client.AuthenticationException.

https://github.com/raju-balpande/apache_ozone/actions/runs/7741058283/job/21107319150

adoroszlai · 2024-02-01T17:34:28Z

Thanks @raju-balpande for the patch.

Raju Balpande added 6 commits January 31, 2024 15:35

HDDS-10165. Replace GenericTestUtils temp dirs with in ozone unit tests

faaedc7

HDDS-10165. Replace GenericTestUtils temp dirs with in ozone unit tests

bc3608f

HDDS-10165. Replace GenericTestUtils temp dirs with in ozone unit tests

16a173e

HDDS-10165. Replace GenericTestUtils temp dirs with in ozone unit tests

aea0336

HDDS-10165. Replace GenericTestUtils temp dirs with in ozone unit tests

dab9d35

HDDS-10165. Replace GenericTestUtils temp dirs with in ozone unit tests

9cb5cfd

adoroszlai reviewed Feb 1, 2024

View reviewed changes

Raju Balpande and others added 2 commits February 1, 2024 17:32

addressed the review comments

daad5f1

Merge branch 'master' into raju-b-hdds-10165

b6279de

HDDS-10165. Replace GenericTestUtils temp dirs with in ozone unit tests

16e6d9e

adoroszlai approved these changes Feb 1, 2024

View reviewed changes

adoroszlai merged commit d5742ef into apache:master Feb 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

HDDS-10165. Replace GenericTestUtils temp dirs with `@TempDir` in ozone unit tests #6142

HDDS-10165. Replace GenericTestUtils temp dirs with `@TempDir` in ozone unit tests #6142

Uh oh!

raju-balpande commented Feb 1, 2024 •

edited by adoroszlai

Loading

Uh oh!

adoroszlai commented Feb 1, 2024 •

edited

Loading

Uh oh!

adoroszlai Feb 1, 2024

Uh oh!

raju-balpande Feb 1, 2024

Uh oh!

adoroszlai Feb 1, 2024

Uh oh!

raju-balpande Feb 1, 2024

Uh oh!

adoroszlai Feb 1, 2024

Uh oh!

raju-balpande Feb 1, 2024

Uh oh!

adoroszlai Feb 1, 2024

Uh oh!

raju-balpande Feb 1, 2024

Uh oh!

adoroszlai commented Feb 1, 2024

Uh oh!

adoroszlai commented Feb 1, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@BeforeAll public static void setUp() throws Exception {
		File base = new File(BASEDIR);

	File idFile = baseDir.resolve("/tokenFile").toFile();
	File idFile = baseDir.resolve("tokenFile").toFile();

HDDS-10165. Replace GenericTestUtils temp dirs with @TempDir in ozone unit tests #6142

HDDS-10165. Replace GenericTestUtils temp dirs with @TempDir in ozone unit tests #6142

Uh oh!

Conversation

raju-balpande commented Feb 1, 2024 • edited by adoroszlai Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

adoroszlai commented Feb 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adoroszlai commented Feb 1, 2024

Uh oh!

adoroszlai commented Feb 1, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

HDDS-10165. Replace GenericTestUtils temp dirs with `@TempDir` in ozone unit tests #6142

HDDS-10165. Replace GenericTestUtils temp dirs with `@TempDir` in ozone unit tests #6142

raju-balpande commented Feb 1, 2024 •

edited by adoroszlai

Loading

adoroszlai commented Feb 1, 2024 •

edited

Loading