Skip to content

test: de-flake scheduled ITs (test-planted handle collisions, leaked bitstream formats, stale Solr searcher) #1413

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
milanmajchrak wants to merge 2 commits into
base: dtq-dev
Choose a base branch
from
Open

test: de-flake scheduled ITs (test-planted handle collisions, leaked bitstream formats, stale Solr searcher) #1413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2d60931
test: use explicit handles the sequence can never mint
milanmajchrak Aug 18, 2026
3899124
test: stop bitstream-format leak, de-randomize format names, commit v…
milanmajchrak Aug 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 3 additions & 8 deletions dspace-api/src/test/java/org/dspace/curate/ItemHandleCheckerIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import java.io.IOException;
import java.sql.SQLException;
import java.util.List;
import java.util.Random;

import okhttp3.mockwebserver.Dispatcher;
import okhttp3.mockwebserver.MockResponse;
Expand Down Expand Up @@ -58,7 +57,9 @@
public class ItemHandleCheckerIT extends AbstractIntegrationTestWithDatabase {
private static final String TASK_NAME = "checkhandles";

private static final String HANDLE_COLLECTION = "123456789/" + randomString();
// Non-numeric suffix the handle sequence can never mint, unique to this class so it cannot
// clash with leftover handle rows of other tests either.
private static final String HANDLE_COLLECTION = "123456789/handle-checker-test";

private static final String HANDLE_ITEM1 = HANDLE_COLLECTION + "-1";
private static final String HANDLE_ITEM2 = HANDLE_COLLECTION + "-2";
Expand Down Expand Up @@ -280,10 +281,4 @@ private String getIdentifierUri(Item item) {
private List<MetadataValue> getIdentifierUris(Item item) {
return itemService.getMetadata(item, "dc", "identifier", "uri", Item.ANY);
}

private static String randomString() {
Random r = new Random();
// Generate random integers in range 1000 to 1999
return String.valueOf(1000 + r.nextInt(1000));
}
}
11 changes: 3 additions & 8 deletions dspace-api/src/test/java/org/dspace/curate/RequiredMetadataIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

import java.io.IOException;
import java.sql.SQLException;
import java.util.Random;

import org.dspace.AbstractIntegrationTestWithDatabase;
import org.dspace.authorize.AuthorizeException;
Expand Down Expand Up @@ -44,7 +43,9 @@
public class RequiredMetadataIT extends AbstractIntegrationTestWithDatabase {
private static final String TASK_NAME = "requiredmetadata";

private static final String HANDLE_COLLECTION = "123456789/" + randomString();
// Non-numeric suffix the handle sequence can never mint, unique to this class so it cannot
// clash with leftover handle rows of other tests either.
private static final String HANDLE_COLLECTION = "123456789/required-metadata-test";
private static final String HANDLE_ITEM1 = HANDLE_COLLECTION + "-1";
private static final String HANDLE_ITEM2 = HANDLE_COLLECTION + "-2";
private static final String HANDLE_ITEM3 = HANDLE_COLLECTION + "-3";
Expand Down Expand Up @@ -154,10 +155,4 @@ private static String successResultForItem(Item item) {
return "Item: " + item.getHandle() + " has all required fields";
}

private static String randomString() {
Random r = new Random();
// Generate random integers in range 1000 to 1999
return String.valueOf(1000 + r.nextInt(1000));
}

}
4 changes: 3 additions & 1 deletion dspace-api/src/test/java/org/dspace/xmlworkflow/XmlWorkflowFactoryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ public void init() {
this.owningCommunity = communityService.create(null, context);
this.mappedCollection =
this.collectionService.create(context, owningCommunity, "123456789/workflow-test-1");
this.nonMappedCollection = this.collectionService.create(context, owningCommunity, "123456789/999");
// Non-numeric suffix so the handle sequence can never mint a colliding handle
this.nonMappedCollection =
this.collectionService.create(context, owningCommunity, "123456789/workflow-test-2");
//we need to commit the changes so we don't block the table for testing
context.restoreAuthSystemState();
} catch (SQLException e) {
Expand Down
69 changes: 44 additions & 25 deletions dspace-server-webapp/src/test/java/org/dspace/app/rest/BitstreamFormatRestRepositoryIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@

import java.util.Arrays;
import java.util.Map;
import java.util.Random;
import java.util.concurrent.atomic.AtomicReference;

import com.fasterxml.jackson.databind.ObjectMapper;
Expand All @@ -39,7 +38,9 @@
import org.dspace.eperson.EPerson;
import org.hamcrest.Matchers;
import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TestName;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.test.web.servlet.MvcResult;

Expand All @@ -56,7 +57,8 @@ public class BitstreamFormatRestRepositoryIT extends AbstractControllerIntegrati
@Autowired
private BitstreamFormatConverter bitstreamFormatConverter;

private final int DEFAULT_AMOUNT_FORMATS = 95;
@Rule
public TestName testName = new TestName();

@Test
public void findAllPaginationTest() throws Exception {
Expand Down Expand Up @@ -136,7 +138,7 @@ public void findOneNonExistentIDInURL() throws Exception {
@Test
public void createAdminAccess() throws Exception {
ObjectMapper mapper = new ObjectMapper();
BitstreamFormatRest bitstreamFormatRest = this.createRandomMockBitstreamRest(false);
BitstreamFormatRest bitstreamFormatRest = this.createMockBitstreamRest();

//Create bitstream format
String token = getAuthToken(admin.getEmail(), password);
Expand All @@ -145,11 +147,16 @@ public void createAdminAccess() throws Exception {
AtomicReference<Integer> idRef = new AtomicReference<>();
try {

// Capture the id on the POST itself, so the format is cleaned up even when a later
// assertion in this test fails.
MvcResult mvcResult = getClient(token).perform(post("/api/core/bitstreamformats/")
.content(mapper.writeValueAsBytes(bitstreamFormatRest))
.contentType(contentType))
.andExpect(status().isCreated())
.andExpect(jsonPath("$", HalMatcher.matchNoEmbeds()))
.andDo(result -> idRef
.set(read(result.getResponse().getContentAsString(),
"$.id")))
.andReturn();

String content = mvcResult.getResponse().getContentAsString();
Expand All @@ -168,13 +175,13 @@ public void createAdminAccess() throws Exception {
bitstreamFormatRest.getMimetype(),
bitstreamFormatRest.getDescription(),
bitstreamFormatRest.getShortDescription())
)))
.andDo(result -> idRef
.set(read(result.getResponse().getContentAsString(), "$.id")));
)));

} finally {
// Delete the created community (cleanup after ourselves!)
BitstreamFormatBuilder.deleteBitstreamFormat(idRef.get());
// Delete the created bitstream format (cleanup after ourselves!)
if (idRef.get() != null) {
BitstreamFormatBuilder.deleteBitstreamFormat(idRef.get());
}
}


Expand All @@ -183,8 +190,9 @@ public void createAdminAccess() throws Exception {
@Test
public void createNonValidSupportLevel() throws Exception {
ObjectMapper mapper = new ObjectMapper();
BitstreamFormatRest bitstreamFormatRest = this.createRandomMockBitstreamRest(false);
BitstreamFormatRest bitstreamFormatRest = this.createMockBitstreamRest();
bitstreamFormatRest.setSupportLevel("NONVALID SUPPORT LVL");
int totalFormatsBefore = currentTotalFormats();
//Attempt to create bitstream with a non-valid support lvl
String token = getAuthToken(admin.getEmail(), password);
getClient(token).perform(post("/api/core/bitstreamformats/")
Expand All @@ -194,13 +202,14 @@ public void createNonValidSupportLevel() throws Exception {
// Check that no new bitstreamformat was created
getClient().perform(get("/api/core/bitstreamformats/"))
.andExpect(status().isOk())
.andExpect(jsonPath("$.page.totalElements", is(DEFAULT_AMOUNT_FORMATS)));
.andExpect(jsonPath("$.page.totalElements", is(totalFormatsBefore)));
}

@Test
public void createNoAccess() throws Exception {
ObjectMapper mapper = new ObjectMapper();
BitstreamFormatRest bitstreamFormatRest = this.createRandomMockBitstreamRest(false);
BitstreamFormatRest bitstreamFormatRest = this.createMockBitstreamRest();
int totalFormatsBefore = currentTotalFormats();

//Try to create bitstreamFormat without auth token
getClient(null).perform(post("/api/core/bitstreamformats/")
Expand All @@ -210,13 +219,14 @@ public void createNoAccess() throws Exception {
// Check that no new bitstreamformat was created
getClient().perform(get("/api/core/bitstreamformats/"))
.andExpect(status().isOk())
.andExpect(jsonPath("$.page.totalElements", is(DEFAULT_AMOUNT_FORMATS)));
.andExpect(jsonPath("$.page.totalElements", is(totalFormatsBefore)));
}

@Test
public void createNonAdminAccess() throws Exception {
ObjectMapper mapper = new ObjectMapper();
BitstreamFormatRest bitstreamFormatRest = this.createRandomMockBitstreamRest(false);
BitstreamFormatRest bitstreamFormatRest = this.createMockBitstreamRest();
int totalFormatsBefore = currentTotalFormats();
context.turnOffAuthorisationSystem();
EPerson user = EPersonBuilder.createEPerson(context)
.withNameInMetadata("first", "last")
Expand All @@ -234,13 +244,14 @@ public void createNonAdminAccess() throws Exception {
// Check that no new bitstreamformat was created
getClient().perform(get("/api/core/bitstreamformats/"))
.andExpect(status().isOk())
.andExpect(jsonPath("$.page.totalElements", is(DEFAULT_AMOUNT_FORMATS)));
.andExpect(jsonPath("$.page.totalElements", is(totalFormatsBefore)));
}

@Test
public void createAlreadyExisting() throws Exception {
ObjectMapper mapper = new ObjectMapper();
BitstreamFormatRest bitstreamFormatRest = this.createRandomMockBitstreamRest(true);
BitstreamFormatRest bitstreamFormatRest = this.createMockBitstreamRest();
int totalFormatsBefore = currentTotalFormats();

// Capture the Id of the created BitstreamFormat (see andDo() below)
AtomicReference<Integer> idRef = new AtomicReference<>();
Expand All @@ -264,12 +275,14 @@ public void createAlreadyExisting() throws Exception {
// Check that the new bitstreamformat was created only once
getClient().perform(get("/api/core/bitstreamformats/"))
.andExpect(status().isOk())
.andExpect(jsonPath("$.page.totalElements", is(DEFAULT_AMOUNT_FORMATS + 1)));
.andExpect(jsonPath("$.page.totalElements", is(totalFormatsBefore + 1)));


} finally {
// Delete the created community (cleanup after ourselves!)
BitstreamFormatBuilder.deleteBitstreamFormat(idRef.get());
// Delete the created bitstream format (cleanup after ourselves!)
if (idRef.get() != null) {
Comment thread
milanmajchrak marked this conversation as resolved.
BitstreamFormatBuilder.deleteBitstreamFormat(idRef.get());
}
}
}

Expand Down Expand Up @@ -645,19 +658,25 @@ public void deleteNonAdminAccess() throws Exception {
)));
}

private BitstreamFormatRest createRandomMockBitstreamRest(boolean withRand) {
/**
* Short descriptions are unique in the format registry, so derive them from the test method
* name: deterministic and collision-free, unlike the random numbers used before.
*/
private BitstreamFormatRest createMockBitstreamRest() {
BitstreamFormatRest bitstreamFormatRest = new BitstreamFormatRest();
String random = null;
if (withRand) {
Random rand = new Random();
random = String.valueOf(rand.nextInt(100) + 1);
}
bitstreamFormatRest.setShortDescription("Test short" + random);
bitstreamFormatRest.setShortDescription("Test short " + testName.getMethodName());
bitstreamFormatRest.setDescription("Full description of Test short");
bitstreamFormatRest.setMimetype("text/plain");
bitstreamFormatRest.setSupportLevel("KNOWN");
bitstreamFormatRest.setInternal(false);
bitstreamFormatRest.setExtensions(Arrays.asList("txt", "asc"));
return bitstreamFormatRest;
}

private int currentTotalFormats() throws Exception {
String response = getClient().perform(get("/api/core/bitstreamformats/"))
.andExpect(status().isOk())
.andReturn().getResponse().getContentAsString();
return read(response, "$.page.totalElements");
}
}
11 changes: 8 additions & 3 deletions dspace-server-webapp/src/test/java/org/dspace/app/rest/PreviewContentServiceImplIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.dspace.app.rest.test.AbstractControllerIntegrationTest;
import org.dspace.authorize.AuthorizeException;
import org.dspace.builder.BitstreamBuilder;
import org.dspace.builder.BitstreamFormatBuilder;
import org.dspace.builder.BundleBuilder;
import org.dspace.builder.CollectionBuilder;
import org.dspace.builder.CommunityBuilder;
Expand Down Expand Up @@ -256,11 +257,15 @@ public void destroy() throws Exception {
BitstreamBuilder.deleteBitstream(tarXzFileWithIncorrectMimeType.getID());

// removing custom mime type format created for tarXGzipFile and tgzFileWithGzipMimeType files
if (customMimeTypeFormat != null) {
bitstreamFormatService.delete(context, customMimeTypeFormat);
}
Integer customMimeTypeFormatId = customMimeTypeFormat == null ? null : customMimeTypeFormat.getID();

super.destroy();

// Must run in its own Context after super.destroy(): a delete through the shared test
// context is never committed here, which leaked one format row per test.
if (customMimeTypeFormatId != null) {
BitstreamFormatBuilder.deleteBitstreamFormat(customMimeTypeFormatId);
}
}

@Test
Expand Down
4 changes: 4 additions & 0 deletions dspace-server-webapp/src/test/java/org/dspace/app/rest/StatisticsRestRepositoryIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,10 @@ public void totalVisitsReport_Item_Visited() throws Exception {
.contentType(contentType))
.andExpect(status().isCreated());

// Commit the view event waiting for a new searcher so it is visible to the report query
// below (same race and fix as in topCountriesReport_Community_Visited).
StatisticsServiceFactory.getInstance().getSolrLoggerService().commit();

// And request that collection's TotalVisits stat report
getClient(adminToken).perform(
get("/api/statistics/usagereports/" + itemVisited.getID() + "_" + TOTAL_VISITS_REPORT_ID))
Expand Down
Loading