chore(backend): fix some more racy backend tests by not asserting on …

…exact order (#3247)

* chore(backend): deflake ApproveProcessedDataEndpointTests using order independent helper

I keep getting some flakes due to exact order requirements in tests.

So I've now added a helper that makes it easy to test for accession versions being present in any order.

Using this consistently in one test file where I've gotten a few flakes recently, but could probably be rolled out elsewhere as well.

* Deflake DeleteSequencesEndpointTests with order independent assertions

backend/src/test/kotlin/org/loculus/backend/controller/TestHelpers.kt

@@ -9,6 +9,9 @@ import kotlinx.datetime.plus
 import kotlinx.datetime.toLocalDateTime
 import org.hamcrest.MatcherAssert.assertThat
 import org.hamcrest.Matchers
+import org.hamcrest.Matchers.allOf
+import org.hamcrest.Matchers.containsInAnyOrder
+import org.hamcrest.Matchers.hasEntry
 import org.hamcrest.Matchers.`is`
 import org.hamcrest.Matchers.not
 import org.loculus.backend.api.AccessionVersion
@@ -19,8 +22,10 @@ import org.loculus.backend.api.Status
 import org.loculus.backend.utils.DateProvider
 import org.springframework.test.web.servlet.MvcResult
 import org.springframework.test.web.servlet.ResultActions
+import org.springframework.test.web.servlet.ResultMatcher
 import org.springframework.test.web.servlet.result.MockMvcResultMatchers
 import org.springframework.test.web.servlet.result.MockMvcResultMatchers.content
+import org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath
 import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status
 import org.testcontainers.shaded.org.awaitility.Awaitility.await
 
@@ -35,6 +40,16 @@ fun AccessionVersionInterface.toAccessionVersion() = AccessionVersion(this.acces
 
 fun List<AccessionVersionInterface>.getAccessionVersions() = map { it.toAccessionVersion() }
 
+fun List<AccessionVersionInterface>.toAccessionVersionMatcher() = map { entry ->
+    allOf(
+        hasEntry("accession", entry.accession),
+        hasEntry("version", entry.version.toInt()),
+    )
+}
+
+fun jsonContainsAccessionVersionsInAnyOrder(expectedVersions: List<AccessionVersionInterface>): ResultMatcher =
+    jsonPath("\$[*]", containsInAnyOrder(expectedVersions.toAccessionVersionMatcher()))
+
 fun addOrganismToPath(path: String, organism: String = DEFAULT_ORGANISM) = "/$organism/${path.trimStart('/')}"
 
 val jacksonObjectMapper: ObjectMapper = jacksonObjectMapper().findAndRegisterModules()

backend/src/test/kotlin/org/loculus/backend/controller/submission/ApproveProcessedDataEndpointTest.kt

@@ -2,7 +2,6 @@ package org.loculus.backend.controller.submission
 
 import org.hamcrest.MatcherAssert.assertThat
 import org.hamcrest.Matchers.containsString
-import org.hamcrest.Matchers.hasItem
 import org.hamcrest.Matchers.hasSize
 import org.hamcrest.Matchers.`is`
 import org.junit.jupiter.api.Test
@@ -22,6 +21,7 @@ import org.loculus.backend.controller.assertStatusIs
 import org.loculus.backend.controller.expectUnauthorizedResponse
 import org.loculus.backend.controller.generateJwtFor
 import org.loculus.backend.controller.getAccessionVersions
+import org.loculus.backend.controller.jsonContainsAccessionVersionsInAnyOrder
 import org.loculus.backend.controller.jwtForSuperUser
 import org.loculus.backend.controller.submission.SubmitFiles.DefaultFiles.NUMBER_OF_SEQUENCES
 import org.springframework.beans.factory.annotation.Autowired
@@ -54,7 +54,7 @@ class ApproveProcessedDataEndpointTest(
         client.approveProcessedSequenceEntries(scope = ALL, accessionVersions)
             .andExpect(status().isOk)
             .andExpect(content().contentType(MediaType.APPLICATION_JSON))
-            .andExpect(jsonPath("\$[*].accession").value(accessionVersions.map { it.accession }))
+            .andExpect(jsonContainsAccessionVersionsInAnyOrder(accessionVersions))
 
         assertThat(
             convenienceClient.getSequenceEntries().statusCounts[APPROVED_FOR_RELEASE],
@@ -69,7 +69,7 @@ class ApproveProcessedDataEndpointTest(
         client.approveProcessedSequenceEntries(scope = ALL, accessionVersions)
             .andExpect(status().isOk)
             .andExpect(content().contentType(MediaType.APPLICATION_JSON))
-            .andExpect(jsonPath("\$[*].accession").value(accessionVersions.map { it.accession }))
+            .andExpect(jsonContainsAccessionVersionsInAnyOrder(accessionVersions))
 
         assertThat(
             convenienceClient.getSequenceEntries().statusCounts[APPROVED_FOR_RELEASE],
@@ -79,12 +79,12 @@ class ApproveProcessedDataEndpointTest(
 
     @Test
     fun `WHEN I approve without accession filter or with full scope THEN all data is approved`() {
-        val approvableSequences = convenienceClient.prepareDataTo(PROCESSED).map { it.accession }
+        val accessionVersions = convenienceClient.prepareDataTo(PROCESSED)
 
         client.approveProcessedSequenceEntries(scope = ALL)
             .andExpect(status().isOk)
             .andExpect(content().contentType(MediaType.APPLICATION_JSON))
-            .andExpect(jsonPath("\$[*].accession").value(approvableSequences))
+            .andExpect(jsonContainsAccessionVersionsInAnyOrder(accessionVersions))
 
         assertThat(
             convenienceClient.getSequenceEntriesOfUserInState(status = APPROVED_FOR_RELEASE),
@@ -117,31 +117,32 @@ class ApproveProcessedDataEndpointTest(
     fun `WHEN I approve a sequence entry that does not exist THEN no accession should be approved`() {
         val nonExistentAccession = "999"
 
-        val accessionVersions = convenienceClient.prepareDataTo(PROCESSED).getAccessionVersions()
+        val processedAccessionVersion = convenienceClient.prepareDataTo(PROCESSED).getAccessionVersions().first()
 
         client.approveProcessedSequenceEntries(
             scope = ALL,
             listOf(
-                accessionVersions.first(),
+                processedAccessionVersion,
                 AccessionVersion(nonExistentAccession, 1),
             ),
         )
             .andExpect(status().isUnprocessableEntity)
             .andExpect(content().contentType(MediaType.APPLICATION_JSON))
-            .andExpect(jsonPath("$.detail", containsString("Accession versions 999.1 do not exist")))
+            .andExpect(jsonPath("$.detail", containsString("Accession versions $nonExistentAccession.1 do not exist")))
 
-        convenienceClient.getSequenceEntry(accessionVersions.first()).assertStatusIs(PROCESSED)
+        convenienceClient.getSequenceEntry(processedAccessionVersion).assertStatusIs(PROCESSED)
     }
 
     @Test
     fun `WHEN I approve a sequence entry that does not exist THEN no sequence should be approved`() {
         val accessionVersions = convenienceClient.prepareDataTo(PROCESSED).getAccessionVersions()
         val nonExistingVersion = accessionVersions[1].copy(version = 999L)
+        val processedAccessionVersion = accessionVersions.first()
 
         client.approveProcessedSequenceEntries(
             scope = ALL,
             listOf(
-                accessionVersions.first(),
+                processedAccessionVersion,
                 nonExistingVersion,
             ),
         )
@@ -157,7 +158,7 @@ class ApproveProcessedDataEndpointTest(
                 ),
             )
 
-        convenienceClient.getSequenceEntry(accessionVersions.first()).assertStatusIs(PROCESSED).assertHasError(false)
+        convenienceClient.getSequenceEntry(processedAccessionVersion).assertStatusIs(PROCESSED).assertHasError(false)
     }
 
     @Test
@@ -240,8 +241,7 @@ class ApproveProcessedDataEndpointTest(
         )
             .andExpect(status().isOk)
             .andExpect(content().contentType(MediaType.APPLICATION_JSON))
-            .andExpect(jsonPath("$.[*]", hasSize<List<*>>(otherOrganismData.size)))
-            .andExpect(jsonPath("$.[*].accession", hasItem(otherOrganismData.first().accession)))
+            .andExpect(jsonContainsAccessionVersionsInAnyOrder(otherOrganismData))
 
         convenienceClient.getSequenceEntry(
             accession = defaultOrganismData.first().accession,
@@ -357,8 +357,7 @@ class ApproveProcessedDataEndpointTest(
 
         client.approveProcessedSequenceEntries(scope = ALL, jwt = jwtForSuperUser)
             .andExpect(status().isOk)
-            .andExpect(jsonPath("\$.length()").value(accessionVersions.size))
-            .andExpect(jsonPath("\$[*].accession").value(accessionVersions.map { it.accession }))
+            .andExpect(jsonContainsAccessionVersionsInAnyOrder(accessionVersions))
 
         convenienceClient.getSequenceEntry(accessionVersions.first()).assertStatusIs(APPROVED_FOR_RELEASE)
     }
@@ -379,8 +378,7 @@ class ApproveProcessedDataEndpointTest(
             submitterNamesFilter = listOf(DEFAULT_USER_NAME),
         )
             .andExpect(status().isOk)
-            .andExpect(jsonPath("\$.length()").value(accessionVersions.size))
-            .andExpect(jsonPath("\$[*].accession").value(accessionVersions.map { it.accession }))
+            .andExpect(jsonContainsAccessionVersionsInAnyOrder(accessionVersions))
 
         convenienceClient.getSequenceEntry(accessionVersions.first()).assertStatusIs(APPROVED_FOR_RELEASE)
         convenienceClient.getSequenceEntry(
@@ -402,8 +400,7 @@ class ApproveProcessedDataEndpointTest(
             jwt = jwtForSuperUser,
         )
             .andExpect(status().isOk)
-            .andExpect(jsonPath("\$.length()").value(accessionVersions.size))
-            .andExpect(jsonPath("\$[*].accession").value(accessionVersions.map { it.accession }))
+            .andExpect(jsonContainsAccessionVersionsInAnyOrder(accessionVersions))
 
         convenienceClient.getSequenceEntry(accessionVersions.first()).assertStatusIs(APPROVED_FOR_RELEASE)
     }

backend/src/test/kotlin/org/loculus/backend/controller/submission/DeleteSequencesEndpointTest.kt

@@ -1,11 +1,8 @@
 package org.loculus.backend.controller.submission
 
 import org.hamcrest.MatcherAssert.assertThat
-import org.hamcrest.Matchers.allOf
-import org.hamcrest.Matchers.containsInAnyOrder
 import org.hamcrest.Matchers.containsString
 import org.hamcrest.Matchers.equalTo
-import org.hamcrest.Matchers.hasEntry
 import org.hamcrest.Matchers.hasItem
 import org.hamcrest.Matchers.hasProperty
 import org.hamcrest.Matchers.hasSize
@@ -28,6 +25,7 @@ import org.loculus.backend.controller.OTHER_ORGANISM
 import org.loculus.backend.controller.assertStatusIs
 import org.loculus.backend.controller.expectUnauthorizedResponse
 import org.loculus.backend.controller.generateJwtFor
+import org.loculus.backend.controller.jsonContainsAccessionVersionsInAnyOrder
 import org.loculus.backend.controller.jwtForSuperUser
 import org.loculus.backend.controller.submission.SubmitFiles.DefaultFiles.NUMBER_OF_SEQUENCES
 import org.loculus.backend.controller.toAccessionVersion
@@ -79,11 +77,7 @@ class DeleteSequencesEndpointTest(
         deletionResult
             .andExpect(status().isOk)
             .andExpect(content().contentType(MediaType.APPLICATION_JSON_VALUE))
-            .andExpect(jsonPath("\$.length()").value(accessionVersionsToDelete.size))
-
-        accessionVersionsToDelete.forEach {
-            deletionResult.andExpect(jsonPath("\$[*].accession", hasItem(it.accession)))
-        }
+            .andExpect(jsonContainsAccessionVersionsInAnyOrder(accessionVersionsToDelete))
 
         assertThat(
             convenienceClient.getSequenceEntriesOfUserInState(
@@ -157,14 +151,16 @@ class DeleteSequencesEndpointTest(
         val erroneousSequences = convenienceClient.prepareDataTo(Status.PROCESSED, errors = true)
         val approvableSequences = convenienceClient.prepareDataTo(Status.PROCESSED)
 
+        val allSequences = erroneousSequences + approvableSequences
+
         assertThat(
             convenienceClient.getSequenceEntries().sequenceEntries,
-            hasSize(erroneousSequences.size + approvableSequences.size),
+            hasSize(allSequences.size),
         )
 
         client.deleteSequenceEntries(scope = ALL)
             .andExpect(status().isOk)
-            .andExpect(jsonPath("\$.length()").value(2 * NUMBER_OF_SEQUENCES))
+            .andExpect(jsonContainsAccessionVersionsInAnyOrder(allSequences))
 
         assertThat(
             convenienceClient.getSequenceEntries().sequenceEntries,
@@ -193,7 +189,7 @@ class DeleteSequencesEndpointTest(
 
         client.deleteSequenceEntries(scope = DeleteSequenceScope.PROCESSED_WITH_ERRORS)
             .andExpect(status().isOk)
-            .andExpect(jsonPath("\$.length()").value(NUMBER_OF_SEQUENCES))
+            .andExpect(jsonContainsAccessionVersionsInAnyOrder(erroneousSequences))
 
         assertThat(
             convenienceClient.getProcessingResultCount(ProcessingResult.HAS_ERRORS),
@@ -242,8 +238,7 @@ class DeleteSequencesEndpointTest(
         )
             .andExpect(status().isOk)
             .andExpect(content().contentType(MediaType.APPLICATION_JSON_VALUE))
-            .andExpect(jsonPath("$.[*]", hasSize<List<*>>(otherOrganismData.size)))
-            .andExpect(jsonPath("$.[*].accession", hasItem(otherOrganismData.first().accession)))
+            .andExpect(jsonContainsAccessionVersionsInAnyOrder(otherOrganismData))
 
         convenienceClient.getSequenceEntry(
             accession = defaultOrganismData.first().accession,
@@ -299,9 +294,7 @@ class DeleteSequencesEndpointTest(
         client.deleteSequenceEntries(scope = ALL, jwt = jwtForSuperUser)
             .andExpect(status().isOk)
             .andExpect(content().contentType(MediaType.APPLICATION_JSON_VALUE))
-            .andExpect(jsonPath("\$.length()").value(accessionVersions.size))
-            .andExpect(jsonPath("\$[0].accession").value(accessionVersions.first().accession))
-            .andExpect(jsonPath("\$[0].version").value(accessionVersions.first().version))
+            .andExpect(jsonContainsAccessionVersionsInAnyOrder(accessionVersions))
     }
 
     @Test
@@ -312,22 +305,14 @@ class DeleteSequencesEndpointTest(
             )
             .submissionIdMappings
 
-        val expectedPairs = accessionVersions.map { entry ->
-            allOf(
-                hasEntry("accession", entry.accession),
-                hasEntry("version", entry.version.toInt()),
-            )
-        }
-
         client.deleteSequenceEntries(
             scope = ALL,
             accessionVersionsFilter = accessionVersions,
             jwt = jwtForSuperUser,
         )
             .andExpect(status().isOk)
             .andExpect(content().contentType(MediaType.APPLICATION_JSON_VALUE))
-            .andExpect(jsonPath("\$.length()").value(accessionVersions.size))
-            .andExpect(jsonPath("\$[*]", containsInAnyOrder(expectedPairs)))
+            .andExpect(jsonContainsAccessionVersionsInAnyOrder(accessionVersions))
     }
 
     @Test