hemantk-12 commented on code in PR #5518:
URL: https://github.com/apache/ozone/pull/5518#discussion_r1389075350


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotFileSystem.java:
##########
@@ -101,65 +101,46 @@ public class TestOmSnapshotFileSystem {
   private static FileSystem fs;
   private static OzoneFileSystem o3fs;
   private static OzoneManagerProtocol writeClient;
-  private static BucketLayout bucketLayout;
-  private static boolean enabledFileSystemPaths;
   private static OzoneManager ozoneManager;
   private static String keyPrefix;
 
   private static final Logger LOG =
       LoggerFactory.getLogger(TestOmSnapshot.class);
 
-  @Rule
-  public TestRule timeout = new JUnit5AwareTimeout(Timeout.seconds(120));
-
-  @Parameterized.Parameters
-  public static Collection<Object[]> data() {
-    return Arrays.asList(
-        new Object[]{BucketLayout.FILE_SYSTEM_OPTIMIZED, false},
-        new Object[]{BucketLayout.LEGACY, true});
-  }
-
-  public TestOmSnapshotFileSystem(BucketLayout newBucketLayout,
-      boolean newEnableFileSystemPaths) throws Exception {
-    // Checking whether 'newBucketLayout' and
-    // 'newEnableFileSystemPaths' flags represents next parameter
-    // index values. This is to ensure that initialize init() function
-    // will be invoked only at the beginning of every new set of
-    // Parameterized.Parameters.
-    if (TestOmSnapshotFileSystem.enabledFileSystemPaths !=
-        newEnableFileSystemPaths ||
-        TestOmSnapshotFileSystem.bucketLayout != newBucketLayout) {
-      setConfig(newBucketLayout, newEnableFileSystemPaths);
-      tearDown();
-      init();
-    }
-  }
-
-  private static void setConfig(BucketLayout newBucketLayout,
-      boolean newEnableFileSystemPaths) {
-    TestOmSnapshotFileSystem.enabledFileSystemPaths = newEnableFileSystemPaths;
-    TestOmSnapshotFileSystem.bucketLayout = newBucketLayout;
+  public static Stream<Arguments> bucketLayoutType() {
+    return Stream.of(
+        Arguments.of(BucketLayout.FILE_SYSTEM_OPTIMIZED),
+        Arguments.of(BucketLayout.LEGACY)
+    );
   }
 
-  /**
-   * Create a MiniDFSCluster for testing.
-   */
-  private static void init() throws Exception {
+  @BeforeAll
+  public static void init() throws Exception {
     conf = new OzoneConfiguration();
     String clusterId = UUID.randomUUID().toString();
     String scmId = UUID.randomUUID().toString();
     String omId = UUID.randomUUID().toString();
-    conf.setBoolean(OMConfigKeys.OZONE_OM_ENABLE_FILESYSTEM_PATHS,
-        enabledFileSystemPaths);
-    conf.set(OMConfigKeys.OZONE_DEFAULT_BUCKET_LAYOUT,
-        bucketLayout.name());
+    conf.setBoolean(OMConfigKeys.OZONE_OM_ENABLE_FILESYSTEM_PATHS, true);
     cluster = MiniOzoneCluster.newBuilder(conf).setClusterId(clusterId)
         .setScmId(scmId).setOmId(omId).build();
     cluster.waitForClusterToBeReady();
     client = cluster.newClient();
+
+    objectStore = client.getObjectStore();
+    writeClient = objectStore.getClientProxy().getOzoneManagerClient();
+    ozoneManager = cluster.getOzoneManager();
+
+    // stop the deletion services so that keys can still be read
+    KeyManagerImpl keyManager = (KeyManagerImpl) ozoneManager.getKeyManager();
+    keyManager.stop();
+  }
+
+  /**
+   * Create a MiniDFSCluster for testing.
+   */
+  private static void setup(BucketLayout layoutType) throws Exception {
     // create a volume and a bucket to be used by OzoneFileSystem
-    OzoneBucket bucket = TestDataUtil
-        .createVolumeAndBucket(client, bucketLayout);
+    OzoneBucket bucket = TestDataUtil.createVolumeAndBucket(client, 
layoutType);

Review Comment:
   The reason, it was done this way because `fs` and `ofs` are based on bucket 
name.
   
   I looked if there is any way a to use parameterized `@BeforeEach` and pass 
bucket name but currently Junit-5 doesn't support that and there have been some 
discussions (https://github.com/junit-team/junit5/issues/871 and 
https://github.com/junit-team/junit5/issues/878) going on around it. I came 
across 
https://code-case.hashnode.dev/how-to-pass-parameterized-test-parameters-to-beforeeachaftereach-method-in-junit5
 and seems like a good way to make @BeforeEach parameterized. I was thinking to 
implement this and create generic util but it should not be part of this PR.
   
   For the sake of this PR, I took simple approach and implemented 
https://github.com/junit-team/junit5/issues/871#issuecomment-1330899418 for our 
usecase.
   
   But will discuss the idea of generic util in next upcoming community sync or 
wait for Junit to implement 
https://github.com/junit-team/junit5/issues/878#issuecomment-1509853864. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to