Skip to content

Commit

Permalink
fix: only restrict set of checks when executing them [DHIS2-16256] (#…
Browse files Browse the repository at this point in the history
…15840)

* fix: only restrict set of checks when executing them [DHIS2-16256]

* test: show that slow tests are included in summary [DHIS2-16256]
  • Loading branch information
jbee committed Dec 6, 2023
1 parent 29f45b4 commit 00f50c8
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -853,7 +853,7 @@ public Collection<DataIntegrityCheck> getDataIntegrityChecks(Set<String> checks)
ensureConfigurationsAreLoaded();
return checks.isEmpty()
? unmodifiableCollection(checksByName.values())
: expandChecks(checks).stream().map(checksByName::get).collect(toList());
: expandChecks(checks, false).stream().map(checksByName::get).collect(toList());
}

@Nonnull
Expand All @@ -868,7 +868,7 @@ public Map<String, DataIntegritySummary> getSummaries(@Nonnull Set<String> check
public void runSummaryChecks(@Nonnull Set<String> checks, JobProgress progress) {
runDataIntegrityChecks(
"Data Integrity summary checks",
expandChecks(checks),
expandChecks(checks, true),
progress,
summaryCache,
runningSummaryChecks,
Expand All @@ -890,7 +890,7 @@ public Map<String, DataIntegrityDetails> getDetails(@Nonnull Set<String> checks,
public void runDetailsChecks(@Nonnull Set<String> checks, JobProgress progress) {
runDataIntegrityChecks(
"Data Integrity details checks",
expandChecks(checks),
expandChecks(checks, true),
progress,
detailsCache,
runningDetailsChecks,
Expand All @@ -907,7 +907,7 @@ private static String errorMessage(DataIntegrityCheck check, RuntimeException ex
}

private <T> Map<String, T> getCached(Set<String> checks, long timeout, Cache<T> cache) {
Set<String> names = expandChecks(checks);
Set<String> names = expandChecks(checks, false);
long giveUpTime = currentTimeMillis() + timeout;
Map<String, T> resByName = new LinkedHashMap<>();
boolean retry = false;
Expand Down Expand Up @@ -975,11 +975,11 @@ private <T> void runDataIntegrityChecks(
}
}

private Set<String> expandChecks(Set<String> names) {
private Set<String> expandChecks(Set<String> names, boolean restricted) {
ensureConfigurationsAreLoaded();

if (CollectionUtils.isEmpty(names)) {
return getDefaultChecks();
return getDefaultChecks(restricted);
}
Set<String> expanded = new LinkedHashSet<>();

Expand Down Expand Up @@ -1008,13 +1008,15 @@ private Set<String> expandChecks(Set<String> names) {
return expanded;
}

private Set<String> getDefaultChecks() {
private Set<String> getDefaultChecks(boolean restricted) {
ensureConfigurationsAreLoaded();

Predicate<DataIntegrityCheck> filter =
restricted ? not(DataIntegrityCheck::isSlow) : check -> true;
return checksByName.values().stream()
.filter(not(DataIntegrityCheck::isSlow))
.filter(filter)
.map(DataIntegrityCheck::getName)
.collect(Collectors.toUnmodifiableSet());
.collect(toUnmodifiableSet());
}

private void ensureConfigurationsAreLoaded() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.hisp.dhis.web.HttpStatus;
import org.hisp.dhis.webapi.DhisControllerIntegrationTest;
import org.hisp.dhis.webapi.json.domain.JsonDataIntegrityDetails;
import org.hisp.dhis.webapi.json.domain.JsonDataIntegrityDetails.JsonDataIntegrityIssue;
import org.hisp.dhis.webapi.json.domain.JsonDataIntegritySummary;
import org.hisp.dhis.webapi.json.domain.JsonWebMessage;
import org.springframework.beans.factory.annotation.Autowired;
Expand Down Expand Up @@ -89,7 +90,7 @@ void checkDataIntegritySummary(
}
}

private Boolean hasComments(JsonList<JsonDataIntegrityDetails.JsonDataIntegrityIssue> issues) {
private Boolean hasComments(JsonList<JsonDataIntegrityIssue> issues) {
return issues.stream()
.map(issue -> issue.has("comment"))
.reduce(Boolean.FALSE, Boolean::logicalOr);
Expand All @@ -104,7 +105,7 @@ void checkDataIntegrityDetailsIssues(
postDetails(check);

JsonDataIntegrityDetails details = getDetails(check);
JsonList<JsonDataIntegrityDetails.JsonDataIntegrityIssue> issues = details.getIssues();
JsonList<JsonDataIntegrityIssue> issues = details.getIssues();
assertTrue(issues.exists());
assertEquals(1, issues.size());

Expand Down Expand Up @@ -137,13 +138,14 @@ void checkDataIntegrityDetailsIssues(
postDetails(check);

JsonDataIntegrityDetails details = getDetails(check);
JsonList<JsonDataIntegrityDetails.JsonDataIntegrityIssue> issues = details.getIssues();
JsonList<JsonDataIntegrityIssue> issues = details.getIssues();

assertTrue(issues.exists());
assertEquals(expectedDetailsUnits.size(), issues.size());

/* Always check the UIDs */
Set<String> issueUIDs = issues.stream().map(issue -> issue.getId()).collect(Collectors.toSet());
Set<String> issueUIDs =
issues.stream().map(JsonDataIntegrityIssue::getId).collect(Collectors.toSet());
assertEquals(issueUIDs, expectedDetailsUnits);

/*
Expand All @@ -152,7 +154,7 @@ void checkDataIntegrityDetailsIssues(
*/
if (!expectedDetailsNames.isEmpty()) {
Set<String> detailsNames =
issues.stream().map(issue -> issue.getName()).collect(Collectors.toSet());
issues.stream().map(JsonDataIntegrityIssue::getName).collect(Collectors.toSet());
assertEquals(expectedDetailsNames, detailsNames);
}
/* This can be empty if comments do not exist in the JSON response. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ void testGetAvailableChecks() {
for (DataIntegrityCheckType type : DataIntegrityCheckType.values()) {
assertCheckExists(type.getName(), checks);
}
checks.stream()
.filter(JsonDataIntegrityCheck::getIsSlow)
.findFirst()
.orElseThrow(() -> new AssertionError("There should be slow tests"));
}

@Test
Expand Down

0 comments on commit 00f50c8

Please sign in to comment.