Merge pull request #133 from GuyPaddock/feature/issue-132--let-should-be-lazy

Fix Issue #132 -- Make `let` Lazily Evaluated

Author: greghaskins

docs/VariablesAndValues.md

@@ -12,10 +12,14 @@ when the test is broken into separate steps.
 The `let` function is used to initialise a fresh, isolated, object for each spec.
 
 ### Common Variable Initialization
+#### Let
+The `let` helper function makes it easy to initialize common variables that are used in multiple
+specs. In standard JUnit you might expect to use the initializer list of the class or a `@Before`
+method to achieve the same. As there is no easy way for `beforeAll` or `beforeEach` to instantiate
+a value that will be used in the specs, `let` is the tool of choice.
 
-The `let` helper function makes it easy to initialize common variables that are used in multiple specs. In standard JUnit you might expect to use the initializer list of the class or a `@Before`	method to achieve the same. As there is no easy way for `beforeAll` or `beforeEach` to instantiate a value that will be used in the specs, `let` is the tool of choice.
-
-Values are cached within a spec, and lazily re-initialized between specs as in [RSpec #let](http://rspec.info/documentation/3.5/rspec-core/RSpec/Core/MemoizedHelpers/ClassMethods.html#let-instance_method).
+Values are cached within a spec, and lazily re-initialized between specs as in
+[RSpec #let](http://rspec.info/documentation/3.5/rspec-core/RSpec/Core/MemoizedHelpers/ClassMethods.html#let-instance_method).
 
 > from [LetSpecs.java](../src/test/java/specs/LetSpecs.java)
 
@@ -42,7 +46,64 @@ describe("The `let` helper function", () -> {
 });
 ```
 
-For cases where you need to access a shared variable across specs or steps, the `Variable` helper class provides a simple `get`/`set` interface. This may be required, for example, to initialize shared state in a `beforeAll` that is used across multiple specs in that suite. Of course, you should exercise caution when sharing state across tests
+#### Eager Let
+If you need to ensure that a value is initialized at the start of a test, you can use the `eagerLet`
+helper function, which has the same semantics as `let` but is evaluated prior to `beforeEach`. This
+is often useful when you need to initialize values you can use in your `beforeEach` block. The value
+is still initialized after any `beforeAll` blocks.
+
+This is similar to
+[RSpec #let!](http://rspec.info/documentation/3.5/rspec-core/RSpec/Core/MemoizedHelpers/ClassMethods.html#let!-instance_method).
+
+> from [EagerLetSpecs.java](../src/test/java/specs/EagerLetSpecs.java)
+
+```java
+describe("The `eagerLet` helper function", () -> {
+  final Supplier<List<String>> items = eagerLet(() -> new ArrayList<>(asList("foo", "bar")));
+
+  final Supplier<List<String>> eagerItemsCopy = eagerLet(() -> new ArrayList<>(items.get()));
+
+  context("when `beforeEach`, `let`, and `eagerLet` are used", () -> {
+    final Supplier<List<String>> lazyItemsCopy =
+      let(() -> new ArrayList<>(items.get()));
+
+    beforeEach(() -> {
+      // This would throw a NullPointerException if it ran before eagerItems
+      items.get().add("baz");
+    });
+
+    it("evaluates all `eagerLet` blocks at once", () -> {
+      assertThat(eagerItemsCopy.get(), contains("foo", "bar"));
+    });
+
+    it("evaluates `beforeEach` after `eagerLet`", () -> {
+      assertThat(items.get(), contains("foo", "bar", "baz"));
+    });
+
+    it("evaluates `let` upon first use", () -> {
+      assertThat(lazyItemsCopy.get(), contains("foo", "bar", "baz"));
+    });
+  });
+
+  context("when `beforeAll` and `eagerLet` are used", () -> {
+    beforeAll(() -> {
+      assertThat(items.get(), is(nullValue()));
+      assertThat(eagerItemsCopy.get(), is(nullValue()));
+    });
+
+    it("evaluates `beforeAll` prior to `eagerLet`", () -> {
+      assertThat(items.get(), is(not(nullValue())));
+      assertThat(eagerItemsCopy.get(), is(not(nullValue())));
+    });
+  });
+});
+```
+
+#### Variable
+For cases where you need to access a shared variable across specs or steps, the `Variable` helper
+class provides a simple `get`/`set` interface. This may be required, for example, to initialize
+shared state in a `beforeAll` that is used across multiple specs in that suite. Of course, you
+should exercise caution when sharing state across tests
 
 > from [VariableSpecs.java](../src/test/java/specs/VariableSpecs.java)
 

src/main/java/com/greghaskins/spectrum/dsl/specification/Specification.java

@@ -12,6 +12,7 @@
 import com.greghaskins.spectrum.internal.DeclarationState;
 import com.greghaskins.spectrum.internal.Suite;
 import com.greghaskins.spectrum.internal.blocks.IdempotentBlock;
+import com.greghaskins.spectrum.internal.hooks.EagerLetHook;
 import com.greghaskins.spectrum.internal.hooks.Hook;
 import com.greghaskins.spectrum.internal.hooks.HookContext.AppliesTo;
 import com.greghaskins.spectrum.internal.hooks.HookContext.Precedence;
@@ -185,6 +186,26 @@ static <T> Supplier<T> let(final ThrowingSupplier<T> supplier) {
     return letHook;
   }
 
+  /**
+   * A value that will be calculated fresh at the start of each spec and cannot bleed across specs.
+   *
+   * <p>
+   * Note that {@code eagerLet} is eagerly evaluated: the {@code supplier} is called at the start
+   * of the spec, before {@code beforeEach} blocks.
+   * </p>
+   *
+   * @param <T>      The type of value
+   * @param supplier {@link ThrowingSupplier} function that either generates the value, or throws a
+   *                 {@link Throwable}
+   * @return supplier which is refreshed for each spec's context
+   */
+  static <T> Supplier<T> eagerLet(final ThrowingSupplier<T> supplier) {
+    EagerLetHook<T> eagerLetHook = new EagerLetHook<>(supplier);
+    DeclarationState.instance().addHook(eagerLetHook, AppliesTo.ATOMIC_ONLY, Precedence.LOCAL);
+
+    return eagerLetHook;
+  }
+
   /**
    * Define a test context. Alias for {@link #describe}.
    *

src/main/java/com/greghaskins/spectrum/internal/hooks/EagerLetHook.java

@@ -0,0 +1,26 @@
+package com.greghaskins.spectrum.internal.hooks;
+
+import com.greghaskins.spectrum.ThrowingSupplier;
+
+/**
+ * Implementation of an eager version of {@code let}.
+ *
+ * <p>Sematics are the same as with {@link LetHook}, except that all values are calculated at the
+ * start of the test, rather than on an as-needed basis.
+ */
+public class EagerLetHook<T> extends AbstractSupplyingHook<T> {
+  private final ThrowingSupplier<T> supplier;
+
+  public EagerLetHook(final ThrowingSupplier<T> supplier) {
+    this.supplier = supplier;
+  }
+
+  protected T before() {
+    return supplier.get();
+  }
+
+  protected String getExceptionMessageIfUsedAtDeclarationTime() {
+    return "Cannot use the value from eagerLet() in a suite declaration. "
+        + "It may only be used in the context of a running spec.";
+  }
+}

src/main/java/com/greghaskins/spectrum/internal/hooks/LetHook.java

@@ -1,24 +1,77 @@
 package com.greghaskins.spectrum.internal.hooks;
 
+import com.greghaskins.spectrum.Block;
 import com.greghaskins.spectrum.ThrowingSupplier;
+import com.greghaskins.spectrum.Variable;
+import com.greghaskins.spectrum.internal.DeclarationState;
+import com.greghaskins.spectrum.internal.RunReporting;
+
+import org.junit.runner.Description;
+import org.junit.runner.notification.Failure;
 
 /**
- * Implementation of let as a supplying hook.
+ * Implementation of {@code let} as a supplying hook.
+ *
+ * <p>Using {@code let} allows you to define shared values that can be used by multiple tests,
+ * without having to worry about cleaning up the values between tests to prevent shared state in
+ * one test from affecting the results of another.
+ *
+ * <p>Values are lazily initialized and then cached, so a value is not calculated until the first
+ * time it is needed in a given test. Subsequent fetches of the value within the same test will
+ * return the cached value.
  */
-public class LetHook<T> extends AbstractSupplyingHook<T> {
-
+public class LetHook<T> implements SupplyingHook<T> {
   private final ThrowingSupplier<T> supplier;
+  private final Variable<T> cachedValue = new Variable<>();
+  private boolean isCached;
 
   public LetHook(final ThrowingSupplier<T> supplier) {
     this.supplier = supplier;
+    this.isCached = false;
   }
 
-  protected T before() {
-    return supplier.get();
+  @Override
+  public void accept(final Description description,
+      final RunReporting<Description, Failure> reporting, final Block block)
+      throws Throwable {
+    try {
+      block.run();
+    } finally {
+      clear();
+    }
+  }
+
+  @Override
+  public T get() {
+    assertSpectrumIsRunningTestsNotDeclaringThem();
+
+    if (!this.isCached) {
+      this.cachedValue.set(supplier.get());
+
+      this.isCached = true;
+    }
+
+    return this.cachedValue.get();
   }
 
   protected String getExceptionMessageIfUsedAtDeclarationTime() {
     return "Cannot use the value from let() in a suite declaration. "
         + "It may only be used in the context of a running spec.";
   }
+
+  private void clear() {
+    this.isCached = false;
+    this.cachedValue.set(null);
+  }
+
+  /**
+   * Will throw an exception if this method happens to be called while Spectrum is still defining
+   * tests, rather than executing them. Useful to see if a hook is being accidentally used during
+   * definition.
+   */
+  private void assertSpectrumIsRunningTestsNotDeclaringThem() {
+    if (DeclarationState.instance().getCurrentSuiteBeingDeclared() != null) {
+      throw new IllegalStateException(getExceptionMessageIfUsedAtDeclarationTime());
+    }
+  }
 }