Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: let() is invoked before beforeEach #132

Closed
GuyPaddock opened this issue Mar 8, 2018 · 1 comment
Closed

Bug: let() is invoked before beforeEach #132

GuyPaddock opened this issue Mar 8, 2018 · 1 comment

Comments

@GuyPaddock
Copy link
Contributor

GuyPaddock commented Mar 8, 2018

In RSpec, let blocks are evaluated and cached lazily in each spec only when their value is first needed within a given test. But, in Spectrum, it appears that the let blocks all fire off and get cached at the start of the test. In fact, they all actually seem to get invoked before any beforeEach blocks. This means that let blocks that depend on any state that the beforeEach blocks manipulate will not be correct.

Here is a test that demonstrates the issue:

package com.rosieapp.test;

import static com.greghaskins.spectrum.dsl.specification.Specification.beforeEach;
import static com.greghaskins.spectrum.dsl.specification.Specification.context;
import static com.greghaskins.spectrum.dsl.specification.Specification.it;
import static com.greghaskins.spectrum.dsl.specification.Specification.let;
import static org.assertj.core.api.Assertions.assertThat;

import com.greghaskins.spectrum.Spectrum;
import com.greghaskins.spectrum.Variable;
import java.util.function.Supplier;
import org.junit.runner.RunWith;

@RunWith(Spectrum.class)
public class BadTest {
  {
    final Variable<String> theVariable = new Variable<>();
    final Supplier<String> result      = let(() -> theVariable.get());

    context("when `theVariable` is `x`", () -> {
      beforeEach(() -> {
        theVariable.set("x");
      });

      it("stores `x` in `result`", () -> {
        // Fails: `result` is `null` because it was fetched & cached before the `beforeEach`
        assertThat(result.get()).isEqualTo("x");
      });
    });

    context("when `theVariable` is `y`", () -> {
      beforeEach(() -> {
        theVariable.set("y");
      });

      it("stores `y` in `result`", () -> {
        // Fails: `result` is `x` because it was fetched & cached before the `beforeEach`
        assertThat(result.get()).isEqualTo("y");
      });
    });
  }
}

To work around this limitation, I'm having to use beforeAll() at the start of each context. That works (and is a bit more efficient since we only have to reset the variable once per context), but introduces the possibility that each of the tests within a context are sharing more state than desired.

@GuyPaddock
Copy link
Contributor Author

This sounds related to #127, though I'm using specification syntax rather than Gherkin.

GuyPaddock pushed a commit to GuyPaddock/spectrum that referenced this issue Mar 8, 2018
The previous implementation would cause all `let` blocks to be evaluated at the start of the test rather than on an as-needed basis, which is inconsistent with the behavior developers who are used to RSpec would expect. Now, `let` only evaluates its block when the test first needs a value, allowing the `let` block to depend on state that is being setup by other blocks (`beforeEach`, other `let` blocks, etc).
GuyPaddock pushed a commit to GuyPaddock/spectrum that referenced this issue Mar 8, 2018
The previous implementation would cause all `let` blocks to be evaluated at the start of the test rather than on an as-needed basis, which is inconsistent with the behavior developers who are used to RSpec would expect. Now, `let` only evaluates its block when the test first needs a value, allowing the `let` block to depend on state that is being setup by other blocks (`beforeEach`, other `let` blocks, etc).
GuyPaddock pushed a commit to GuyPaddock/spectrum that referenced this issue Mar 8, 2018
The previous implementation is closer to `let!` from RSpec. As such, it's still useful for cases in which a developer needs values to be available in the `beforeEach` block.
GuyPaddock pushed a commit to GuyPaddock/spectrum that referenced this issue Mar 8, 2018
The previous implementation is closer to `let!` from RSpec. As such, it's still useful for cases in which a developer needs values to be available in the `beforeEach` block.
GuyPaddock pushed a commit to GuyPaddock/spectrum that referenced this issue Mar 8, 2018
The previous implementation is closer to `let!` from RSpec. As such, it's still useful for cases in which a developer needs values to be available in the `beforeEach` block.
GuyPaddock pushed a commit to GuyPaddock/spectrum that referenced this issue Mar 8, 2018
The previous implementation is closer to `let!` from RSpec. As such, it's still useful for cases in which a developer needs values to be available in the `beforeEach` block.
greghaskins added a commit that referenced this issue Mar 18, 2018
…-be-lazy

Fix Issue #132 -- Make `let` Lazily Evaluated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant