#patch: #145 make updating snapshots simpler

README.md

@@ -56,6 +56,7 @@ reporters=au.com.origin.snapshots.reporters.PlainTextSnapshotReporter
 snapshot-dir=__snapshots__
 output-dir=src/test/java
 ci-env-var=CI
+update-snapshot=none
 ```
 
 3. Enable snapshot testing and write your first test
@@ -447,17 +448,18 @@ Often your IDE has an excellent file comparison tool.
 
 This file allows you to conveniently setup global defaults
 
-|     key          |  Description                                                                                                                           |
-|------------------|----------------------------------------------------------------------------------------------------------------------------------------|
-|serializer        | Class name of the [serializer](#supplying-a-custom-snapshotserializer), default serializer                                             |
-|serializer.{name} | Class name of the [serializer](#supplying-a-custom-snapshotserializer), accessible via `.serializer("{name}")`                         |
-|comparator        | Class name of the [comparator](#supplying-a-custom-snapshotcomparator)                                                                 |
-|comparator.{name} | Class name of the [comparator](#supplying-a-custom-snapshotcomparator), accessible via `.comparator("{name}")`                         |
-|reporters         | Comma separated list of class names to use as [reporters](#supplying-a-custom-snapshotreporter)                                        |
-|reporters.{name}  | Comma separated list of class names to use as [reporters](#supplying-a-custom-snapshotreporter), accessible via `.reporters("{name}")` |
-|snapshot-dir      | Name of sub-folder holding your snapshots                                                                                              |
-|output-dir        | Base directory of your test files (although it can be a different directory if you want)                                               |
-|ci-env-var        | Name of environment variable used to detect if we are running on a Build Server                                                        |
+|     key          |  Description                                                                                                                                                       |
+|------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+|serializer        | Class name of the [serializer](#supplying-a-custom-snapshotserializer), default serializer                                                                         |
+|serializer.{name} | Class name of the [serializer](#supplying-a-custom-snapshotserializer), accessible via `.serializer("{name}")`                                                     |
+|comparator        | Class name of the [comparator](#supplying-a-custom-snapshotcomparator)                                                                                             |
+|comparator.{name} | Class name of the [comparator](#supplying-a-custom-snapshotcomparator), accessible via `.comparator("{name}")`                                                     |
+|reporters         | Comma separated list of class names to use as [reporters](#supplying-a-custom-snapshotreporter)                                                                    |
+|reporters.{name}  | Comma separated list of class names to use as [reporters](#supplying-a-custom-snapshotreporter), accessible via `.reporters("{name}")`                             |
+|snapshot-dir      | Name of sub-folder holding your snapshots                                                                                                                          |
+|output-dir        | Base directory of your test files (although it can be a different directory if you want)                                                                           |
+|ci-env-var        | Name of environment variable used to detect if we are running on a Build Server                                                                                    |
+|update-snapshot   | Similar to `--updateSnapshot` in [Jest](https://jestjs.io/docs/en/snapshot-testing#updating-snapshots) <br/>[all]=update all snapsohts<br/>[none]=update no snapshots<br/>[MyTest1,MyTest2]=update snapshots in these classes only<br/><br/>*Note: must be set to [none] on CI |
 
 For example:
 
@@ -471,6 +473,7 @@ reporters=au.com.origin.snapshots.reporters.PlainTextSnapshotReporter
 snapshot-dir=__snapshots__
 output-dir=src/test/java
 ci-env-var=CI
+update-snapshot=none
 ```
 
 ## Parameterized tests
@@ -784,36 +787,6 @@ public class JUnit5ResolutionHierarchyExample {
 }
 ```
 
-## Automatically overwriting snapshots via `-PupdateSnapshot=filter`
-
-Often - after analysing each snapshot and verifying it is correct, you will need to override the existing
-snapshots.
-
-Note that you may need to do some Gradle trickery to make this visible to your actual tests
-
-```groovy
-test {
-    systemProperty "updateSnapshot", project.getProperty("updateSnapshot")
-}
-```
-
-Instead of deleting or manually modifying each snapshot you can pass `-PupdateSnapshot` which is equivalent to
-the `--updateSnapshot` flag in [Jest](https://jestjs.io/docs/en/snapshot-testing#updating-snapshots)
-
-#### Update all snapshots automatically
-
-```
--PupdateSnapshot
-```
-
-#### Update selected snapshots only using `filter`
-
-pass the class names you want to update to `filter`
-
-```
--PupdateSnapshot=UserService,PermissionRepository
-```
-
 # Troubleshooting
 
 **I'm seeing this error in my logs**

java-snapshot-testing-core/src/main/java/au/com/origin/snapshots/SnapshotContext.java

@@ -4,6 +4,7 @@
 import au.com.origin.snapshots.comparators.SnapshotComparator;
 import au.com.origin.snapshots.config.SnapshotConfig;
 import au.com.origin.snapshots.exceptions.ReservedWordException;
+import au.com.origin.snapshots.exceptions.SnapshotExtensionException;
 import au.com.origin.snapshots.exceptions.SnapshotMatchException;
 import au.com.origin.snapshots.reporters.SnapshotReporter;
 import au.com.origin.snapshots.serializers.SnapshotSerializer;
@@ -118,6 +119,12 @@ public void toMatchSnapshot() {
   }
 
   private boolean shouldUpdateSnapshot() {
+    if (snapshotConfig.updateSnapshot().isPresent() && snapshotConfig.isCI()) {
+      throw new SnapshotExtensionException(
+          "isCI=true & update-snapshot="
+              + snapshotConfig.updateSnapshot()
+              + ". Updating snapshots on CI is not allowed");
+    }
     if (snapshotConfig.updateSnapshot().isPresent()) {
       return resolveSnapshotIdentifier().contains(snapshotConfig.updateSnapshot().get());
     } else {
@@ -126,9 +133,11 @@ private boolean shouldUpdateSnapshot() {
   }
 
   private Snapshot getRawSnapshot(Collection<Snapshot> rawSnapshots) {
-    for (Snapshot rawSnapshot : rawSnapshots) {
-      if (rawSnapshot.getIdentifier().equals(resolveSnapshotIdentifier())) {
-        return rawSnapshot;
+    synchronized (rawSnapshots) {
+      for (Snapshot rawSnapshot : rawSnapshots) {
+        if (rawSnapshot.getIdentifier().equals(resolveSnapshotIdentifier())) {
+          return rawSnapshot;
+        }
       }
     }
     return null;

java-snapshot-testing-core/src/main/java/au/com/origin/snapshots/SnapshotFile.java

@@ -108,11 +108,13 @@ public synchronized File createFileIfNotExists(String filename) {
     return path.toFile();
   }
 
-  public synchronized void pushSnapshot(Snapshot snapshot) {
-    snapshots.add(snapshot);
-    TreeSet<String> rawSnapshots =
-        snapshots.stream().map(Snapshot::raw).collect(Collectors.toCollection(TreeSet::new));
-    updateFile(this.fileName, rawSnapshots);
+  public void pushSnapshot(Snapshot snapshot) {
+    synchronized (snapshots) {
+      snapshots.add(snapshot);
+      TreeSet<String> rawSnapshots =
+          snapshots.stream().map(Snapshot::raw).collect(Collectors.toCollection(TreeSet::new));
+      updateFile(this.fileName, rawSnapshots);
+    }
   }
 
   public synchronized void pushDebugSnapshot(Snapshot snapshot) {

java-snapshot-testing-core/src/main/java/au/com/origin/snapshots/config/PropertyResolvingSnapshotConfig.java

@@ -2,10 +2,15 @@
 
 import au.com.origin.snapshots.SnapshotProperties;
 import au.com.origin.snapshots.comparators.SnapshotComparator;
+import au.com.origin.snapshots.exceptions.MissingSnapshotPropertiesKeyException;
+import au.com.origin.snapshots.logging.LoggingHelper;
 import au.com.origin.snapshots.reporters.SnapshotReporter;
 import au.com.origin.snapshots.serializers.SnapshotSerializer;
 import java.util.List;
+import java.util.Optional;
+import lombok.extern.slf4j.Slf4j;
 
+@Slf4j
 public class PropertyResolvingSnapshotConfig implements SnapshotConfig {
 
   @Override
@@ -18,6 +23,34 @@ public String getSnapshotDir() {
     return SnapshotProperties.getOrThrow("snapshot-dir");
   }
 
+  @Override
+  public Optional<String> updateSnapshot() {
+    // This was the original way to update snapshots
+    Optional<String> legacyFlag =
+        Optional.ofNullable(System.getProperty(JVM_UPDATE_SNAPSHOTS_PARAMETER));
+    if (legacyFlag.isPresent()) {
+      LoggingHelper.deprecatedV5(
+          log,
+          "Passing -PupdateSnapshot will be removed in a future release. Consider using snapshot.properties 'update-snapshot' toggle instead");
+      return legacyFlag;
+    }
+
+    try {
+      String updateSnapshot = SnapshotProperties.getOrThrow("update-snapshot");
+      if ("all".equals(updateSnapshot)) {
+        return Optional.of("");
+      } else if ("none".equals(updateSnapshot)) {
+        return Optional.empty();
+      }
+      return Optional.of(updateSnapshot);
+    } catch (MissingSnapshotPropertiesKeyException ex) {
+      LoggingHelper.deprecatedV5(
+          log,
+          "You do not have 'update-snapshot=none' defined in your snapshot.properties - consider adding it now");
+      return Optional.empty();
+    }
+  }
+
   @Override
   public SnapshotSerializer getSerializer() {
     return SnapshotProperties.getInstance("serializer");

java-snapshot-testing-core/src/main/java/au/com/origin/snapshots/config/SnapshotConfig.java

@@ -13,7 +13,7 @@
  * library
  */
 public interface SnapshotConfig {
-  String JVM_UPDATE_SNAPSHOTS_PARAMETER = "updateSnapshot";
+  @Deprecated String JVM_UPDATE_SNAPSHOTS_PARAMETER = "updateSnapshot";
 
   /**
    * The base directory where files get written (excluding package directories) default:

java-snapshot-testing-core/src/main/java/au/com/origin/snapshots/logging/LoggingHelper.java

@@ -0,0 +1,11 @@
+package au.com.origin.snapshots.logging;
+
+import org.slf4j.Logger;
+
+public class LoggingHelper {
+
+  public static void deprecatedV5(Logger log, String message) {
+    log.warn(
+        "Deprecation Warning:\n " + message + "\n\nThis feature will be removed in version 5.X");
+  }
+}

java-snapshot-testing-core/src/test/java/au/com/origin/snapshots/UpdateSnapshotPropertyTest.java

@@ -16,6 +16,7 @@
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.junit.jupiter.MockitoExtension;
 
+@Deprecated
 @ExtendWith(MockitoExtension.class)
 public class UpdateSnapshotPropertyTest {
 
@@ -72,13 +73,6 @@ void shouldUpdateSnapshot(TestInfo testInfo) throws IOException {
                 + "]");
   }
 
-  @Disabled
-  @Test
-  void shouldUpdateAllSnapshots() throws IOException {
-    System.setProperty(SnapshotConfig.JVM_UPDATE_SNAPSHOTS_PARAMETER, "");
-    // FIXME
-  }
-
   @Test
   void shouldNotUpdateSnapshot(TestInfo testInfo) {
     SnapshotVerifier snapshotVerifier =

java-snapshot-testing-core/src/test/java/au/com/origin/snapshots/UpdateSnapshotTest.java

@@ -0,0 +1,153 @@
+package au.com.origin.snapshots;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import au.com.origin.snapshots.config.BaseSnapshotConfig;
+import au.com.origin.snapshots.config.SnapshotConfig;
+import au.com.origin.snapshots.exceptions.SnapshotMatchException;
+import java.io.File;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Optional;
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+@ExtendWith(MockitoExtension.class)
+public class UpdateSnapshotTest {
+
+  @BeforeEach
+  public void beforeEach() throws Exception {
+    File file =
+        new File("src/test/java/au/com/origin/snapshots/__snapshots__/UpdateSnapshotTest.snap");
+    String content =
+        "au.com.origin.snapshots.UpdateSnapshotTest.canUpdateAllSnapshots=[\n"
+            + "OLD\n"
+            + "]\n"
+            + "\n"
+            + "\n"
+            + "au.com.origin.snapshots.UpdateSnapshotTest.canUpdateClassNameSnapshots=[\n"
+            + "OLD\n"
+            + "]\n"
+            + "\n"
+            + "\n"
+            + "au.com.origin.snapshots.UpdateSnapshotTest.canUpdateNoSnapshots=[\n"
+            + "OLD\n"
+            + "]";
+    Path parentDir = file.getParentFile().toPath();
+    if (!Files.exists(parentDir)) {
+      Files.createDirectories(parentDir);
+    }
+    Files.write(file.toPath(), content.getBytes(StandardCharsets.UTF_8));
+  }
+
+  @Test
+  void canUpdateAllSnapshots(TestInfo testInfo) throws IOException {
+    SnapshotConfig config =
+        new BaseSnapshotConfig() {
+          @Override
+          public Optional<String> updateSnapshot() {
+            return Optional.of("");
+          }
+        };
+    SnapshotVerifier snapshotVerifier =
+        new SnapshotVerifier(config, testInfo.getTestClass().get(), false);
+    Expect expect = Expect.of(snapshotVerifier, testInfo.getTestMethod().get());
+    expect.toMatchSnapshot("NEW");
+    snapshotVerifier.validateSnapshots();
+
+    String content =
+        new String(
+            Files.readAllBytes(
+                Paths.get(
+                    "src/test/java/au/com/origin/snapshots/__snapshots__/UpdateSnapshotTest.snap")),
+            StandardCharsets.UTF_8);
+    Assertions.assertThat(content)
+        .isEqualTo(
+            "au.com.origin.snapshots.UpdateSnapshotTest.canUpdateAllSnapshots=[\n"
+                + "NEW\n"
+                + "]\n"
+                + "\n"
+                + "\n"
+                + "au.com.origin.snapshots.UpdateSnapshotTest.canUpdateClassNameSnapshots=[\n"
+                + "OLD\n"
+                + "]\n"
+                + "\n"
+                + "\n"
+                + "au.com.origin.snapshots.UpdateSnapshotTest.canUpdateNoSnapshots=[\n"
+                + "OLD\n"
+                + "]");
+  }
+
+  @Test
+  void canUpdateNoSnapshots(TestInfo testInfo) {
+    SnapshotConfig config =
+        new BaseSnapshotConfig() {
+          @Override
+          public Optional<String> updateSnapshot() {
+            return Optional.empty();
+          }
+        };
+    SnapshotVerifier snapshotVerifier =
+        new SnapshotVerifier(config, testInfo.getTestClass().get(), false);
+    Expect expect = Expect.of(snapshotVerifier, testInfo.getTestMethod().get());
+    assertThrows(SnapshotMatchException.class, () -> expect.toMatchSnapshot("FOOBAR"));
+  }
+
+  @Test
+  public void canUpdateNewSnapshots() {
+    SnapshotConfig config =
+        new BaseSnapshotConfig() {
+          @Override
+          public Optional<String> updateSnapshot() {
+            return Optional.of("new");
+          }
+        };
+
+    // TODO Pending Implementation
+  }
+
+  @Test
+  public void canUpdateClassNameSnapshots(TestInfo testInfo) throws IOException {
+    SnapshotConfig config =
+        new BaseSnapshotConfig() {
+          @Override
+          public Optional<String> updateSnapshot() {
+            return Optional.of("UpdateSnapshotTest");
+          }
+        };
+    SnapshotVerifier snapshotVerifier =
+        new SnapshotVerifier(config, testInfo.getTestClass().get(), false);
+    Expect expect = Expect.of(snapshotVerifier, testInfo.getTestMethod().get());
+    expect.toMatchSnapshot("NEW");
+    snapshotVerifier.validateSnapshots();
+
+    String content =
+        new String(
+            Files.readAllBytes(
+                Paths.get(
+                    "src/test/java/au/com/origin/snapshots/__snapshots__/UpdateSnapshotTest.snap")),
+            StandardCharsets.UTF_8);
+    Assertions.assertThat(content)
+        .isEqualTo(
+            "au.com.origin.snapshots.UpdateSnapshotTest.canUpdateAllSnapshots=[\n"
+                + "OLD\n"
+                + "]\n"
+                + "\n"
+                + "\n"
+                + "au.com.origin.snapshots.UpdateSnapshotTest.canUpdateClassNameSnapshots=[\n"
+                + "NEW\n"
+                + "]\n"
+                + "\n"
+                + "\n"
+                + "au.com.origin.snapshots.UpdateSnapshotTest.canUpdateNoSnapshots=[\n"
+                + "OLD\n"
+                + "]");
+  }
+}