From 5dde3de27a8baf8d4aee0d18f88e101fab93fb01 Mon Sep 17 00:00:00 2001
From: Lukas Forer <lukas.forer@i-med.ac.at>
Date: Mon, 28 Aug 2023 15:05:32 +0200
Subject: [PATCH] Check for obsolete snapshots only when all tests are green

---
 .../askimed/nf/test/core/AbstractTestSuite.java    | 14 +++++++++++++-
 .../nf/test/core/AnsiTestExecutionListener.java    |  8 ++++----
 .../java/com/askimed/nf/test/core/ITestSuite.java  |  4 ++++
 .../askimed/nf/test/core/TestExecutionEngine.java  |  7 ++++---
 4 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/src/main/java/com/askimed/nf/test/core/AbstractTestSuite.java b/src/main/java/com/askimed/nf/test/core/AbstractTestSuite.java
index b08fe50d..11530578 100644
--- a/src/main/java/com/askimed/nf/test/core/AbstractTestSuite.java
+++ b/src/main/java/com/askimed/nf/test/core/AbstractTestSuite.java
@@ -27,6 +27,8 @@ public abstract class AbstractTestSuite implements ITestSuite {
 
 	private SnapshotFile snapshotFile;
 
+	private boolean failedTests = false;
+
 	private List<String> tags = new Vector<String>();
 
 	@Override
@@ -147,9 +149,19 @@ public SnapshotFile getSnapshot() {
 		}
 		return snapshotFile;
 	}
-	
+
 	@Override
 	public boolean hasSnapshotLoaded() {
 		return (snapshotFile != null);
 	}
+
+	@Override
+	public void setFailedTests(boolean failedTests) {
+		this.failedTests = failedTests;
+	}
+
+	@Override
+	public boolean hasFailedTests() {
+		return failedTests;
+	}
 }
diff --git a/src/main/java/com/askimed/nf/test/core/AnsiTestExecutionListener.java b/src/main/java/com/askimed/nf/test/core/AnsiTestExecutionListener.java
index b6d14424..53fabad1 100644
--- a/src/main/java/com/askimed/nf/test/core/AnsiTestExecutionListener.java
+++ b/src/main/java/com/askimed/nf/test/core/AnsiTestExecutionListener.java
@@ -111,14 +111,14 @@ public void testSuiteExecutionFinished(ITestSuite testSuite) {
 
 		// if we have at least one skipped test, we can not
 		// determine if a snapshot is obsolete.
-		// TODO: add check. if at lest one test failed, same situation.
-		if (testSuite.hasSkippedTests()) {
+		if (testSuite.hasSkippedTests() || testSuite.hasFailedTests()) {
 			System.out.println(AnsiText.padding(
-					"Obsolete snapshots can only be checked if all tests of a file are executed.", 2 * TEST_PADDING));
+					"Obsolete snapshots can only be checked if all tests of a file are executed successful.",
+					2 * TEST_PADDING));
 			return;
 		}
 
-		if (snapshot.getObsoleteSnapshots().size() > 0) {			
+		if (snapshot.getObsoleteSnapshots().size() > 0) {
 			System.out.println(AnsiColors.yellow(AnsiText.padding(
 					snapshot.getObsoleteSnapshots().size() + " obsolete " + snapshot.getObsoleteSnapshots(),
 					2 * TEST_PADDING)));
diff --git a/src/main/java/com/askimed/nf/test/core/ITestSuite.java b/src/main/java/com/askimed/nf/test/core/ITestSuite.java
index 1558c414..59d53ef7 100644
--- a/src/main/java/com/askimed/nf/test/core/ITestSuite.java
+++ b/src/main/java/com/askimed/nf/test/core/ITestSuite.java
@@ -24,6 +24,10 @@ public interface ITestSuite extends ITaggable {
 	
 	public boolean hasSkippedTests();
 	
+	public void setFailedTests(boolean b);
+	
+	public boolean hasFailedTests();
+	
 	public SnapshotFile getSnapshot();
 	
 	public boolean hasSnapshotLoaded();
diff --git a/src/main/java/com/askimed/nf/test/core/TestExecutionEngine.java b/src/main/java/com/askimed/nf/test/core/TestExecutionEngine.java
index ff316637..e4f635fb 100644
--- a/src/main/java/com/askimed/nf/test/core/TestExecutionEngine.java
+++ b/src/main/java/com/askimed/nf/test/core/TestExecutionEngine.java
@@ -201,6 +201,7 @@ public int execute() throws Throwable {
 					result.setThrowable(e);
 					result.setErrorReport(test.getErrorReport());
 					failed = true;
+					testSuite.setFailedTests(true);
 
 				}
 				test.cleanup();
@@ -209,9 +210,9 @@ public int execute() throws Throwable {
 
 			}
 
-			// Remove obsolete snapshots and when no test was skipped.
-			// TODO: removeObsolete snapshots only when no test failed!!
-			if (cleanSnapshot && !testSuite.hasSkippedTests() && testSuite.hasSnapshotLoaded()) {
+			// Remove obsolete snapshots when no test was skipped and no test failed.
+			if (cleanSnapshot && !testSuite.hasSkippedTests() && !testSuite.hasFailedTests()
+					&& testSuite.hasSnapshotLoaded()) {
 				SnapshotFile snapshot = testSuite.getSnapshot();
 				snapshot.removeObsoleteSnapshots();
 				snapshot.save();