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

Consider patch release branch: Gherkin glitches #127

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions src/main/java/com/greghaskins/spectrum/dsl/gherkin/Gherkin.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static com.greghaskins.spectrum.dsl.specification.Specification.describe;
import static com.greghaskins.spectrum.dsl.specification.Specification.it;
import static com.greghaskins.spectrum.internal.Declaration.addComposite;

import com.greghaskins.spectrum.Block;
import com.greghaskins.spectrum.ParameterizedBlock;
Expand All @@ -13,8 +14,6 @@
import com.greghaskins.spectrum.ParameterizedBlock.SixArgBlock;
import com.greghaskins.spectrum.ParameterizedBlock.ThreeArgBlock;
import com.greghaskins.spectrum.ParameterizedBlock.TwoArgBlock;
import com.greghaskins.spectrum.internal.DeclarationState;
import com.greghaskins.spectrum.internal.Suite;

import java.util.Arrays;

Expand Down Expand Up @@ -50,9 +49,7 @@ static void feature(final String featureName, final Block block) {
* @see #then
*/
static void scenario(final String scenarioName, final Block block) {
final Suite suite = DeclarationState.instance().getCurrentSuiteBeingDeclared()
.addCompositeSuite("Scenario: " + scenarioName);
DeclarationState.instance().beginDeclaration(suite, block);
addComposite("Scenario: " + scenarioName, block);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extracted out to a helper as needed below.

}

/**
Expand Down Expand Up @@ -155,14 +152,14 @@ static <T extends ParameterizedBlock> void scenarioOutline(final String name, fi
describe("Scenario outline: " + name, () -> {
describe("Examples:", () -> {
examples.rows().forEach(example -> {
describe(example.toString(), () -> example.runDeclaration(block));
addComposite(example.toString(), () -> example.runDeclaration(block));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

describe did not create a composite test as scenario did

});
});
});
}

/**
* Construct an Examples table for {@link scenarioOutline}. Used this method to compose individual
* Construct an Examples table for {@link #scenarioOutline}. Used this method to compose individual
* rows created with {@link #example} type methods into a type-implicit container. You should try
* to lay out your examples like a data table as that's what they essentially are. Better than
* just providing some primitives in an example block would be to provide some objects with fields
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static com.greghaskins.spectrum.Configure.focus;
import static com.greghaskins.spectrum.Configure.ignore;
import static com.greghaskins.spectrum.Configure.with;
import static com.greghaskins.spectrum.internal.Declaration.addSuiteInternal;
import static com.greghaskins.spectrum.internal.hooks.AfterHook.after;
import static com.greghaskins.spectrum.internal.hooks.BeforeHook.before;

Expand All @@ -19,6 +20,8 @@

import org.junit.AssumptionViolatedException;

import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.Supplier;

public interface Specification {
Expand All @@ -31,11 +34,7 @@ public interface Specification {
* define each expected behavior
*/
static void describe(final String context, final Block block) {
final Suite suite = DeclarationState.instance()
.getCurrentSuiteBeingDeclared()
.addSuite(context);
suite.applyPreconditions(block);
DeclarationState.instance().beginDeclaration(suite, block);
addSuiteInternal(parent -> parent.addSuite(context), block);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There some stuff about creating a suite that was common to more than one implementation - extracted out to a common helper.

}

/**
Expand Down
41 changes: 41 additions & 0 deletions src/main/java/com/greghaskins/spectrum/internal/Declaration.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package com.greghaskins.spectrum.internal;

import com.greghaskins.spectrum.Block;

import java.util.function.Function;

/**
* Helper functions used by {@link com.greghaskins.spectrum.dsl.specification.Specification} and similar
* DSL-specific classes to build specs.
*/
public interface Declaration {

/**
* Declare a general purpose composite test - this is a suite that runs as an atomic test in
* itself. This is the internal implementation for
* {@link com.greghaskins.spectrum.dsl.gherkin.Gherkin#scenario(String, Block)}.
*
* @param context Description of the context for this composite
* @param block {@link Block} with one or more calls to
* {@link com.greghaskins.spectrum.dsl.specification.Specification#it(String, Block) it}
* or its equivalent, that define each expected behavior
*/
static void addComposite(final String context, final Block block) {
addSuiteInternal(parent -> parent.addCompositeSuite(context), block);
}

/**
* Common implementation of adding a suite to the current suite under declaration.
*
* @param suiteCreator specialisation for adding the right sort of suite
* @param block {@link Block} with one or more calls to
* {@link com.greghaskins.spectrum.dsl.specification.Specification#it(String, Block) it}
* or its equivalent, that define each expected behavior
*/
static void addSuiteInternal(final Function<Suite, Suite> suiteCreator, final Block block) {
final Suite suite = suiteCreator.apply(DeclarationState.instance()
.getCurrentSuiteBeingDeclared());
suite.applyPreconditions(block);
DeclarationState.instance().beginDeclaration(suite, block);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The applyPreconditions bit wasn't being applied to gherkin. Similarly, the boilerplate around getCurrentSuite... and beginDeclaration... felt like it should be written once and specialised every time it was used.

}
3 changes: 2 additions & 1 deletion src/main/java/com/greghaskins/spectrum/internal/Suite.java
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@ public void addHook(final HookContext hook) {
}

private Hooks getHooksFor(final Child child) {
Hooks allHooks = this.parent.getInheritableHooks().plus(this.hooks);
Hooks allHooks = isAtomic() ? new Hooks() : this.parent.getInheritableHooks();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the thing which stops an atomic parent conveying its own inherited hooks down to a child.

allHooks = allHooks.plus(this.hooks);

return child.isAtomic() ? allHooks.forAtomic() : allHooks.forNonAtomic();
}
Expand Down
40 changes: 40 additions & 0 deletions src/test/java/specs/GherkinExampleSpecs.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package specs;

import static com.greghaskins.spectrum.Configure.focus;
import static com.greghaskins.spectrum.Configure.with;
import static com.greghaskins.spectrum.Spectrum.let;
import static com.greghaskins.spectrum.dsl.gherkin.Gherkin.and;
import static com.greghaskins.spectrum.dsl.gherkin.Gherkin.feature;
import static com.greghaskins.spectrum.dsl.gherkin.Gherkin.given;
Expand All @@ -10,11 +13,16 @@
import static org.junit.Assert.assertThat;

import com.greghaskins.spectrum.Spectrum;
import com.greghaskins.spectrum.SpectrumHelper;
import com.greghaskins.spectrum.Variable;

import org.junit.runner.Result;
import org.junit.runner.RunWith;

import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Supplier;

@RunWith(Spectrum.class)
public class GherkinExampleSpecs {
Expand Down Expand Up @@ -54,6 +62,38 @@ public class GherkinExampleSpecs {
});
});

Supplier<List<String>> list = let(ArrayList::new);
scenario("behaviour of let in gherkin", () -> {
given("the data is set", () -> {
list.get().add("hello");
});

then("the data is still there", () -> {
assertThat(list.get().get(0), is("hello"));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before the fix, this was failing as the let was being freshened for both given and then

});
});

scenario("application of block configuration", () -> {
Variable<Result> result = new Variable<>();
when("executing a gherkin test suite with a configured block", () -> {
result.set(SpectrumHelper.run(
() -> {
scenario("some scenario", () -> {
then("should have been ignored", () -> {
});
});
scenario("some focused scenario", with(focus(), () -> {
then("should not have been ignored", () -> {
});
}));
}));
});
then("the block configuration will have applied", () -> {
assertThat(result.get().getRunCount(), is(1));
assertThat(result.get().getIgnoreCount(), is(1));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before the fixes, the with(focus()... didn't work for gherkin

});
});

});
}
}
24 changes: 24 additions & 0 deletions src/test/java/specs/ParameterizedExampleSpecs.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package specs;

import static com.greghaskins.spectrum.Spectrum.let;
import static com.greghaskins.spectrum.dsl.gherkin.Gherkin.and;
import static com.greghaskins.spectrum.dsl.gherkin.Gherkin.example;
import static com.greghaskins.spectrum.dsl.gherkin.Gherkin.given;
Expand All @@ -10,6 +11,7 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertTrue;

import com.greghaskins.spectrum.Spectrum;
import com.greghaskins.spectrum.Variable;
Expand All @@ -18,6 +20,10 @@
import jdk.nashorn.api.scripting.NashornScriptEngine;
import org.junit.runner.RunWith;

import java.util.HashSet;
import java.util.Set;
import java.util.function.Supplier;

import javax.script.ScriptEngine;
import javax.script.ScriptEngineManager;
import javax.script.ScriptException;
Expand Down Expand Up @@ -118,6 +124,24 @@ public class ParameterizedExampleSpecs {
example("bye", -1.5))

);

Supplier<Set<String>> set = let(HashSet::new);
scenarioOutline("behaviour of let inside scenario outline",
(first, second) -> {
given("set contains first", () -> {
set.get().add(first);
});
when("set has second added", () -> {
set.get().add(second);
});
then("both are found in the set", () -> {
assertTrue(set.get().contains(first));
assertTrue(set.get().contains(second));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even after one of the fixes, this wasn't going to work because the scenarioOutline wasn't generating actual scenarios...

});
},
withExamples(
example("a", "b"),
example("b", "c")));
}

// dummy class under test
Expand Down