Skip to content

Make randomOrder configuration, allowing tests to be run in any order… #128

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

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
2 changes: 2 additions & 0 deletions docs/Configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,5 @@ In addition to:
there is also:

- `timeout(Duration timeout)` - make the test fail if it takes too long - see [Timeout](Timeout.md)
- `randomOrder()` - put the tests into a different order each run - this outputs the seed used, allowing you to repeat a particular run
- `randomOrder(long seed)` - put the tests into a pseudo-random order, using the given seed to fix that order - most commonly used to diagnose a particular order having an effect on the result
19 changes: 19 additions & 0 deletions src/main/java/com/greghaskins/spectrum/Configure.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.greghaskins.spectrum.internal.DeclarationState;
import com.greghaskins.spectrum.internal.configuration.BlockFocused;
import com.greghaskins.spectrum.internal.configuration.BlockIgnore;
import com.greghaskins.spectrum.internal.configuration.BlockRandomOrder;
import com.greghaskins.spectrum.internal.configuration.BlockTagging;
import com.greghaskins.spectrum.internal.configuration.BlockTimeout;
import com.greghaskins.spectrum.internal.configuration.ConfiguredBlock;
Expand Down Expand Up @@ -109,6 +110,24 @@ static BlockConfigurationChain timeout(Duration timeout) {
return new BlockConfigurationChain().with(new BlockTimeout(timeout));
}

/**
* Apply random order to all parents from this point in the hierarchy down.
* @return a chainable configuration that will apply random test order to all parent nodes below
*/
static BlockConfigurationChain randomOrder() {
return new BlockConfigurationChain().with(new BlockRandomOrder());
}

/**
* Apply random order to all parents from this point in the hierarchy down.
* Using the given seed to make that random order the same every time.
* @param seed a fixed random seed to (temporarily) fix the order for diagnostics.
* @return a chainable configuration that will apply random test order to all parent nodes below
*/
static BlockConfigurationChain randomOrder(long seed) {
return new BlockConfigurationChain().with(new BlockRandomOrder(seed));
}

/**
* Filter which tests in the current suite will run.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package com.greghaskins.spectrum.internal;

/**
* Tagging interface for a {@link Parent} that sequences its children.
*/
public interface ExecutionSequenceApplier {
/**
* Attache the sequencing strategy.
* @param sequencer execution sequencer that orders the children.
*/
void setSequencer(ExecutionSequencer sequencer);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package com.greghaskins.spectrum.internal;

import java.util.List;

/**
* Put the children into an execution order.
*/
@FunctionalInterface
public interface ExecutionSequencer {
ExecutionSequencer DEFAULT = list -> list;

/**
* Apply the ordering to the list of children.
* @param originalOrder order they are stored
* @return re-ordered
*/
List<Child> sequence(List<Child> originalOrder);
}
13 changes: 11 additions & 2 deletions src/main/java/com/greghaskins/spectrum/internal/Suite.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.greghaskins.spectrum.internal;

import static com.greghaskins.spectrum.internal.ExecutionSequencer.DEFAULT;
import static com.greghaskins.spectrum.internal.configuration.BlockConfiguration.merge;

import com.greghaskins.spectrum.Block;
Expand All @@ -18,7 +19,7 @@
import java.util.List;
import java.util.Set;

public class Suite implements Parent, Child {
public class Suite implements Parent, Child, ExecutionSequenceApplier {
private Hooks hooks = new Hooks();

protected final List<Child> children = new ArrayList<>();
Expand All @@ -34,6 +35,8 @@ public class Suite implements Parent, Child {
private BlockConfiguration configuration = BlockConfiguration.defaultConfiguration();
private NameSanitiser nameSanitiser = new NameSanitiser();

private ExecutionSequencer sequencer = DEFAULT;

/**
* The strategy for running the children within the suite.
*/
Expand Down Expand Up @@ -152,6 +155,11 @@ public Hooks getInheritableHooks() {
return this.parent.getInheritableHooks().plus(this.hooks.forAtomic());
}

@Override
public void setSequencer(ExecutionSequencer sequencer) {
this.sequencer = sequencer;
}

/**
* Set the suite to require certain tags of all tests below.
*
Expand Down Expand Up @@ -283,7 +291,8 @@ public void removeAllChildren() {

private static void defaultChildRunner(final Suite suite,
final RunReporting<Description, Failure> reporting) {
suite.children.forEach((child) -> suite.runChild(child, reporting));
suite.sequencer.sequence(suite.children)
.forEach((child) -> suite.runChild(child, reporting));
}

private String sanitise(final String name) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package com.greghaskins.spectrum.internal.configuration;

import com.greghaskins.spectrum.internal.Child;
import com.greghaskins.spectrum.internal.ExecutionSequenceApplier;
import com.greghaskins.spectrum.internal.ExecutionSequencer;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Random;

/**
* Configuration block that randomises the order of tests in their parent.
*/
public class BlockRandomOrder implements BlockConfigurable<BlockTimeout>, ExecutionSequencer {
private Random random;

public BlockRandomOrder() {
this(System.currentTimeMillis());
}

public BlockRandomOrder(long seed) {
random = new Random(seed);
System.out.println("Random execution order set using seed: " + seed);
}

@Override
public boolean inheritedByChild() {
return true;
}

@Override
public void applyTo(Child child, TaggingFilterCriteria state) {
if (child instanceof ExecutionSequenceApplier) {
((ExecutionSequenceApplier) child).setSequencer(this);
}
}

@Override
public BlockConfigurable<BlockTimeout> merge(BlockConfigurable<?> other) {
// my value supersedes any inherited value

return this;
}

@Override
public List<Child> sequence(List<Child> originalOrder) {
List<Child> result = new ArrayList<>(originalOrder);
Collections.shuffle(result, random);

return result;
}
}
120 changes: 120 additions & 0 deletions src/test/java/specs/RandomOrderSpecs.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
package specs;

import static com.greghaskins.spectrum.Configure.randomOrder;
import static com.greghaskins.spectrum.Configure.with;
import static com.greghaskins.spectrum.Spectrum.describe;
import static com.greghaskins.spectrum.Spectrum.it;
import static com.greghaskins.spectrum.dsl.gherkin.Gherkin.*;
import static java.util.stream.Collectors.toList;
import static junit.framework.TestCase.fail;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.core.Is.is;
import static org.junit.Assert.assertThat;

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

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

import java.util.ArrayList;
import java.util.List;

@RunWith(Spectrum.class)
public class RandomOrderSpecs {
{
describe("Random order specs", () -> {
it("can execute at all", () -> {
Result result = SpectrumHelper.run(() -> {
buildSuite(randomOrder());
});
assertThat(result.getRunCount(), is(6));
});

it("will have different execution order each time", () -> {
List<String> originalFailures = failureList(SpectrumHelper.run(() -> {
buildSuite(randomOrder());
}));

// as we are dealing with random numbers, it may take a few goes before a new
// permutation comes up
int iteration = 0;
Result nextResult;
do {
nextResult = SpectrumHelper.run(() -> {
buildSuite(randomOrder());
});
iteration++;
} while (iteration < 100 && failureList(nextResult).equals(originalFailures));

// should not have reached the limit where we gave up finding a new one
assertThat(iteration, not(is(100)));
});

it("can have the same execution order each time with a seed", () -> {
Result result1 = SpectrumHelper.run(() -> {
buildSuite(randomOrder(12345));
});
Result result2 = SpectrumHelper.run(() -> {
buildSuite(randomOrder(12345));
});

assertThat(failureList(result1), is(failureList(result2)));
});

describe("composite tests", with(randomOrder(), () -> {
scenario("a test that relies on order in a random order tree", () -> {
final List<String> strings = new ArrayList<>();
given("first step adds first", () -> {
strings.add("first");
});
when("second step adds second", () -> {
strings.add("second");
});
and("third step adds third", () -> {
strings.add("third");
});
then("the order is correct", () -> {
assertThat(strings, contains("first", "second", "third"));
});
});
}));
});
Copy link
Owner

Choose a reason for hiding this comment

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

@ashleyfrieze Do you imagine needing more than these specs here ☝️ to sufficiently test the Gherkin-style DSL? This scenario seems pretty straightforward to me. I guess you might have a mixed describe where some pieces run in random order, but the right things (given/when/then) stay in the right order.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These do look like they cover the scenario. I've a funny feeling there was something else worrying me about the feature, perhaps relating back to the bug I found in the gherkin tests. If you're going to merge PR #127, why don't we review this again after rebasing?

Copy link
Owner

Choose a reason for hiding this comment

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

Sounds good. We'll look at it again after #127

}

private List<String> failureList(Result result) {
return result.getFailures().stream()
.map(failure -> failure.getDescription().getMethodName())
.collect(toList());
}

private static void buildSuite(BlockConfigurationChain order) {
// each test fails so we can use the failures to determine if they were random
describe("Tests in random order", with(order, () -> {
describe("scramble each level of the hierarchy", () -> {
it("happens whenever", () -> {
fail();
});
it("happens after or before", () -> {
fail();
});
it("can happen when it likes", () -> {
fail();
});
});
describe("make each level of the hierarchy different", () -> {
it("then happens whenever", () -> {
fail();
});
it("then happens after or before", () -> {
fail();
});
it("then can happen when it likes", () -> {
fail();
});
});
}));
}
}