From dea09d3d45594036a1277f0b23955ee95f05e136 Mon Sep 17 00:00:00 2001 From: Dominic Burger Date: Thu, 14 Dec 2023 13:33:49 +0100 Subject: [PATCH 1/3] Verify that log files contain constraint errors --- .../testbed/runner/InterlisValidator.java | 19 +++++++++++++ .../interlis/testbed/runner/Runner.java | 20 +++++++++++-- .../interlis/testbed/runner/Validator.java | 11 +++++++- .../runner/RunnerWithConstraintsTest.java | 28 +++++++++++++++++++ 4 files changed, 74 insertions(+), 4 deletions(-) diff --git a/src/main/java/ch/geowerkstatt/interlis/testbed/runner/InterlisValidator.java b/src/main/java/ch/geowerkstatt/interlis/testbed/runner/InterlisValidator.java index c65eb73..dcb4b81 100644 --- a/src/main/java/ch/geowerkstatt/interlis/testbed/runner/InterlisValidator.java +++ b/src/main/java/ch/geowerkstatt/interlis/testbed/runner/InterlisValidator.java @@ -6,9 +6,11 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.regex.Pattern; public final class InterlisValidator implements Validator { private static final Logger LOGGER = LogManager.getLogger(); + private static final String CONSTRAINT_ERROR_PREFIX = "Error: "; private final TestOptions options; @@ -46,4 +48,21 @@ public boolean validate(Path filePath, Path logFile) throws ValidatorException { throw new ValidatorException(e); } } + + @Override + public boolean containsConstraintError(Path logFile, String constraintName) throws ValidatorException { + var constraintNameWithWordBoundaries = Pattern.compile("\\b" + Pattern.quote(constraintName) + "\\b"); + + try (var lines = Files.lines(logFile)) { + return lines.anyMatch(line -> { + if (line.startsWith(CONSTRAINT_ERROR_PREFIX) && constraintNameWithWordBoundaries.matcher(line).find()) { + LOGGER.info("Found expected error for constraint " + constraintName + " in log file: " + line); + return true; + } + return false; + }); + } catch (IOException e) { + throw new ValidatorException(e); + } + } } diff --git a/src/main/java/ch/geowerkstatt/interlis/testbed/runner/Runner.java b/src/main/java/ch/geowerkstatt/interlis/testbed/runner/Runner.java index 1c0e2f0..c509273 100644 --- a/src/main/java/ch/geowerkstatt/interlis/testbed/runner/Runner.java +++ b/src/main/java/ch/geowerkstatt/interlis/testbed/runner/Runner.java @@ -102,13 +102,27 @@ private boolean mergeAndValidateTransferFiles() throws ValidatorException { } var logFile = mergedFile.getParent().resolve(patchFileNameWithoutExtension + ".log"); - var mergedFileValid = validator.validate(mergedFile, logFile); - if (mergedFileValid) { - LOGGER.error("Validation of " + mergedFile + " was expected to fail but completed successfully."); + var constraintName = patchFile.getParent().getFileName().toString(); + if (!validateMergedFile(mergedFile, logFile, constraintName)) { valid = false; } } return valid; } + + private boolean validateMergedFile(Path mergedFile, Path logFile, String constraintName) throws ValidatorException { + if (validator.validate(mergedFile, logFile)) { + LOGGER.error("Validation of " + mergedFile + " was expected to fail but completed successfully."); + return false; + } + + if (!validator.containsConstraintError(logFile, constraintName)) { + LOGGER.error("Could not verify constraint " + constraintName + " for merged file " + mergedFile + ". Check the log file at " + logFile + " for details."); + return false; + } + + LOGGER.info("Validation of " + mergedFile + " failed as expected."); + return true; + } } diff --git a/src/main/java/ch/geowerkstatt/interlis/testbed/runner/Validator.java b/src/main/java/ch/geowerkstatt/interlis/testbed/runner/Validator.java index 715342c..71e33f2 100644 --- a/src/main/java/ch/geowerkstatt/interlis/testbed/runner/Validator.java +++ b/src/main/java/ch/geowerkstatt/interlis/testbed/runner/Validator.java @@ -8,8 +8,17 @@ public interface Validator { * * @param filePath the path to the file to validate. * @param logFile the path to the log file. - * @return true if the validation was successful, false otherwise. + * @return {@code true} if the validation was successful, {@code false} otherwise. * @throws ValidatorException if the validation could not be performed. */ boolean validate(Path filePath, Path logFile) throws ValidatorException; + + /** + * Checks if the log file contains an error for the provided constraint. + * + * @param logFile the path to the log file. + * @param constraintName the name of the constraint to check. + * @return {@code true} if the log file contains an error for the constraint, {@code false} otherwise. + */ + boolean containsConstraintError(Path logFile, String constraintName) throws ValidatorException; } diff --git a/src/test/java/ch/geowerkstatt/interlis/testbed/runner/RunnerWithConstraintsTest.java b/src/test/java/ch/geowerkstatt/interlis/testbed/runner/RunnerWithConstraintsTest.java index 4890d8a..1d695c9 100644 --- a/src/test/java/ch/geowerkstatt/interlis/testbed/runner/RunnerWithConstraintsTest.java +++ b/src/test/java/ch/geowerkstatt/interlis/testbed/runner/RunnerWithConstraintsTest.java @@ -51,6 +51,7 @@ public void teardown() { public void runMergesAndValidatesPatchFiles() throws ValidatorException { when(validatorMock.validate(eq(BASE_DATA_FILE), any())).thenReturn(true); when(validatorMock.validate(MERGED_FILE, MERGED_LOG_FILE)).thenReturn(false); + when(validatorMock.containsConstraintError(MERGED_LOG_FILE, CONSTRAINT_NAME)).thenReturn(true); var runner = new Runner(options, validatorMock, mergerMock); @@ -107,4 +108,31 @@ public void runFailsIfMergedFileIsValid() throws ValidatorException { verify(mergerMock).merge(eq(BASE_DATA_FILE), eq(PATCH_FILE), eq(MERGED_FILE)); } + + @Test + public void runFailsIfNoConstraintErrorInLog() throws ValidatorException { + when(validatorMock.validate(eq(BASE_DATA_FILE), any())).thenReturn(true); + when(validatorMock.validate(MERGED_FILE, MERGED_LOG_FILE)).thenReturn(false); + when(validatorMock.containsConstraintError(MERGED_LOG_FILE, CONSTRAINT_NAME)).thenReturn(false); + + var runner = new Runner(options, validatorMock, mergerMock); + + var runResult = runner.run(); + + assertFalse(runResult, "Testbed run should have failed if log file does not contain a constraint error."); + + var errors = appender.getErrorMessages(); + assertEquals(1, errors.size(), "One error should have been logged."); + var errorMessage = errors.getFirst(); + var expectedErrorMessageStart = "Could not verify constraint " + CONSTRAINT_NAME + " for merged file"; + assertTrue( + errorMessage.startsWith(expectedErrorMessageStart), + "Expected error to start with: " + expectedErrorMessageStart + ". Actual: '" + errorMessage + "'."); + + verify(validatorMock).validate(eq(BASE_DATA_FILE), any()); + verify(validatorMock).validate(eq(MERGED_FILE), eq(MERGED_LOG_FILE)); + verify(validatorMock).containsConstraintError(eq(MERGED_LOG_FILE), eq(CONSTRAINT_NAME)); + + verify(mergerMock).merge(eq(BASE_DATA_FILE), eq(PATCH_FILE), eq(MERGED_FILE)); + } } From 293c673c06d23ae79c1c951a9b8f8b3736ed23a5 Mon Sep 17 00:00:00 2001 From: Dominic Burger Date: Thu, 14 Dec 2023 13:55:43 +0100 Subject: [PATCH 2/3] Add tests for constraint errors in log files --- src/test/data/validator/logfile.log | 7 +++ .../testbed/runner/ValidatorTest.java | 48 +++++++++++++++++++ 2 files changed, 55 insertions(+) create mode 100644 src/test/data/validator/logfile.log create mode 100644 src/test/java/ch/geowerkstatt/interlis/testbed/runner/ValidatorTest.java diff --git a/src/test/data/validator/logfile.log b/src/test/data/validator/logfile.log new file mode 100644 index 0000000..4715cc1 --- /dev/null +++ b/src/test/data/validator/logfile.log @@ -0,0 +1,7 @@ +Info: validate data... +Info: assume unknown external objects +Info: first validation pass... +Info: second validation pass... +Info: validate mandatory constraint ModelA.TopicA.ClassA.ConstraintA... +Error: line 10: ModelA.TopicA.ClassA: tid 1: Mandatory Constraint ModelA.TopicA.ClassA.ConstraintA is not true. +Info: validate mandatory constraint ModelA.TopicA.ClassA.ConstraintB... diff --git a/src/test/java/ch/geowerkstatt/interlis/testbed/runner/ValidatorTest.java b/src/test/java/ch/geowerkstatt/interlis/testbed/runner/ValidatorTest.java new file mode 100644 index 0000000..4643d15 --- /dev/null +++ b/src/test/java/ch/geowerkstatt/interlis/testbed/runner/ValidatorTest.java @@ -0,0 +1,48 @@ +package ch.geowerkstatt.interlis.testbed.runner; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public final class ValidatorTest { + private static final String CONSTRAINT_A_NAME = "ModelA.TopicA.ClassA.ConstraintA"; + private static final String CONSTRAINT_B_NAME = "ModelA.TopicA.ClassA.ConstraintB"; + private static final Path BASE_PATH = Path.of("src/test/data/validator"); + private static final Path LOG_FILE = BASE_PATH.resolve("logfile.log"); + + private TestOptions options; + + @BeforeEach + public void setup() { + options = new TestOptions(BASE_PATH, Path.of("ilivalidator.jar")); + } + + @Test + public void containsConstraintError() throws ValidatorException { + var validator = new InterlisValidator(options); + + var result = validator.containsConstraintError(LOG_FILE, CONSTRAINT_A_NAME); + + assertTrue(result, "The log file should contain an error for constraint A."); + } + + @Test + public void noErrorForValidConstraint() throws ValidatorException, IOException { + var validator = new InterlisValidator(options); + + var result = validator.containsConstraintError(LOG_FILE, CONSTRAINT_B_NAME); + + assertFalse(result, "The log file should not contain an error for constraint B."); + + try (var lines = Files.lines(LOG_FILE)) { + var hasConstraintInfo = lines.anyMatch(line -> line.startsWith("Info:") && line.contains(CONSTRAINT_B_NAME)); + assertTrue(hasConstraintInfo, "The log file should contain an info message for the constraint."); + } + } +} From c8a1da7073a38703e3cf108ddc8e4c94ed08a573 Mon Sep 17 00:00:00 2001 From: Dominic Burger Date: Mon, 18 Dec 2023 08:32:32 +0100 Subject: [PATCH 3/3] Combine prefix and regex for constraint error --- .../interlis/testbed/runner/InterlisValidator.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/java/ch/geowerkstatt/interlis/testbed/runner/InterlisValidator.java b/src/main/java/ch/geowerkstatt/interlis/testbed/runner/InterlisValidator.java index dcb4b81..0a6e082 100644 --- a/src/main/java/ch/geowerkstatt/interlis/testbed/runner/InterlisValidator.java +++ b/src/main/java/ch/geowerkstatt/interlis/testbed/runner/InterlisValidator.java @@ -10,7 +10,6 @@ public final class InterlisValidator implements Validator { private static final Logger LOGGER = LogManager.getLogger(); - private static final String CONSTRAINT_ERROR_PREFIX = "Error: "; private final TestOptions options; @@ -51,11 +50,11 @@ public boolean validate(Path filePath, Path logFile) throws ValidatorException { @Override public boolean containsConstraintError(Path logFile, String constraintName) throws ValidatorException { - var constraintNameWithWordBoundaries = Pattern.compile("\\b" + Pattern.quote(constraintName) + "\\b"); + var constraintPattern = Pattern.compile("^Error: .*\\b" + Pattern.quote(constraintName) + "\\b"); try (var lines = Files.lines(logFile)) { return lines.anyMatch(line -> { - if (line.startsWith(CONSTRAINT_ERROR_PREFIX) && constraintNameWithWordBoundaries.matcher(line).find()) { + if (constraintPattern.matcher(line).find()) { LOGGER.info("Found expected error for constraint " + constraintName + " in log file: " + line); return true; }