server/src/internalClusterTest/java/org/opensearch/snapshots/ConcurrentSnapshotsIT.java (2)

205-218: Good fix for the race condition, but consider using existing imports.

The stronger ordering check correctly addresses the flaky test by ensuring SnapshotDeletionsInProgress contains the repository before attempting the update. However, the fully qualified names are unnecessary since ClusterState, SnapshotDeletionsInProgress, and Matchers are already imported at the top of the file (lines 44-45, 93).

         // Stronger ordering: wait until cluster state shows the repository is in-use due to deletion
         assertBusy(() -> {
-            final org.opensearch.cluster.ClusterState state = client().admin().cluster().prepareState().get().getState();
-            final org.opensearch.cluster.SnapshotDeletionsInProgress deletions = state.custom(
-                org.opensearch.cluster.SnapshotDeletionsInProgress.TYPE
-            );
+            final ClusterState state = client().admin().cluster().prepareState().get().getState();
+            final SnapshotDeletionsInProgress deletions = state.custom(SnapshotDeletionsInProgress.TYPE);

-            assertThat("SnapshotDeletionsInProgress must be present once delete starts", deletions, org.hamcrest.Matchers.notNullValue());
+            assertThat("SnapshotDeletionsInProgress must be present once delete starts", deletions, notNullValue());
             assertThat(
-                deletions.getEntries().stream().map(org.opensearch.cluster.SnapshotDeletionsInProgress.Entry::repository).toList(),
+                deletions.getEntries().stream().map(SnapshotDeletionsInProgress.Entry::repository).toList(),
                 hasItem(equalTo(repoName))
             );
         });

You'll need to add the notNullValue static import:

import static org.hamcrest.Matchers.notNullValue;

---

224-228: Correct handling of wrapped exceptions.

The approach of catching a generic Exception and unwrapping the root cause is appropriate since the transport layer can wrap the real exception. The instanceOf matcher is already imported at line 93, so the FQN can be simplified.

         // Transport can wrap the real exception; assert on the unwrapped root cause
         final Exception ex = assertThrows(Exception.class, () -> updateRepository(repoName, "mock", newSettings));
         final Throwable cause = org.opensearch.ExceptionsHelper.unwrapCause(ex);
-        assertThat(cause, org.hamcrest.Matchers.instanceOf(IllegalStateException.class));
+        assertThat(cause, instanceOf(IllegalStateException.class));
         assertEquals("trying to modify or unregister repository that is currently used", cause.getMessage());

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro