Support running tests in a random order?

[tjarratt]

Hey friends!

I rather like this project a lot -- it makes it very easy to write BDD style tests in Java. Many thanks for writing and maintaining it.

I care very deeply about test ordering and randomization - rspec, jasmine, cedar, quick, etc all support test randomization as a way of detecting test pollution.

Is this a feature that is already in Spectrum? I couldn't find any documentation or code to that provided any hint as to how to do this, so I have to assume it doesn't work.

Would this be a desired feature, if I possibly found the time to contribute a PR? Any thoughts as to how it might work?

Thank you!

[greghaskins]

I'm glad Spectrum has been helpful for you, @tjarratt . Thanks for the feedback, it helps motivate us to keep working on it.

Spectrum doesn't currently support randomization, you're right about that. I'm definitely open to the idea, though. A PR would be great if you get the time and are interested in seeing this through. In general, our approach has been to follow the example of RSpec and Jasmine, so we'd want Spectrum to behave in a similar way. I often use their examples and even test cases to match behavior.

It looks like both RSpec and Jasmine use CLI flags and config files to activate randomization. How would you propose activating/configuring the random ordering (and seed)? Spectrum currently doesn't have any kind of project-wide config or CLI interface. I imagine you could do it file-by-file with an annotation parameter like @SpectrumOptions(random=true) or something, but it seems like you'd want to activate randomization globally (on CI, for example). Perhaps Spectrum could use an environment variable or system property?

[tjarratt]

Sorry to took me two weeks to get back to this idea, I'm still quite interested in getting a PR together.

As a very-infrequent contributor to Jasmine, it warms my heart to hear that other people in the BDD community look to it's test cases for matching behavior. That is literally awesome. Thank you.

I think you're absolutely right on a few points here

• most use cases need global randomization (especially on CI)
• it could be done file by file, but...
• using a system property would probably be simplest, since it would integrate well with gradle, maven (and presumably sbt, although I haven't used it yet).

So my proposal is to introduce two specific system properties:

spectrum.randomize -- this would be a boolean, defaulting to true
spectrum.seed -- an integer value, defaulting to the current number of seconds since unix epoch

.. and that both the order of the suites, and the order of the specs within a given suite should be randomized. I'll have to look into jasmine/rspec, but I don't think either of those randomize the order of specs across separate suites.

Peeking into the code it seems like introducing a SuiteRandomizer that accepts a Suite and returns back the randomized Suite, and changing Spectrum#run to use that randomizer before running the root suite would be the easiest implementation.

At the risk of asking too many inane questions, is it worth considering breaking out a separate package for this? The current size of the com.greghaskins.spectrum package is starting to feel a little large, and I wonder if this would be a reasonable component to separate out.

[greghaskins]

Sounds like you've got some great ideas. A few thoughts:

• It feels like defaulting to declared order (making randomization opt-in) would make sense, at least at first. That would line up with the default behavior in RSpec and Jasmine (as far as I can tell, I put together a fiddle and random is unchecked by default). We could certainly flip random to being opt-out down the line.
• The system properties you propose sound good. For cases where we want an automatically-defined seed, how can we expose that random seed to the user in case of a failure (or success)? It would be helpful to be able to re-run with the same seed when diagnosing issues.
• SuiteRandomizer sounds like a good approach, and would have minimal impact on the rest of the code. Reading the RSpec docs, it sounds like they put some thought into ordering at each level of nesting. This page on their --order flag, and this one on global ordering might be helpful to us.
• Yep, I think this would warrant a sub-package like com.greghaskins.spectrum.order or something. I mostly just started out small and kept things package-protected for simplicity. Spectrum should probably be organized more cleanly, especially as it grows. To reduce merge conflicts, let's leave the current code mostly in the same place and add that subpackage for ordering/randomization. Then after your PR merges, we can look to reorganize the base code too.

[ashleyfrieze]

#51 contains a SpectrumOptions annotation and there is the start of a method to use system properties as a fallback when options are not provided.

[tjarratt]

@ashleyfrieze thanks for the info! I generally try not to couple pull requests, since that can make it harder for maintainers and contributors.

If your PR finishes up first I'm more than happy to rebase my changes on yours. At the very least, I can plan ahead to use something like SpectrumOptions since that seems wise.

[tjarratt]

@greghaskins the thought of making randomization opt-in makes me a little sad, since I can imagine that few people would think to turn that on, and may never find out their tests are spuriously passing because of test pollution until it's too late.

It's especially strange, because the jasmine source implies that random sorting should be the default case.

However, I'm not fanatical, and I'm happy to talk about changing that behavior after a release, when it may be reasonable to make breaking changes.

[greghaskins]

I hear ya, and I do agree overall. Just trying to balance existing users and expectations from the RSpec/Jasmine world. I'd be open to flipping the default for 2.0. At a minimum, we can put a giant advisory of encouragement in the README.

[greghaskins]

Oh, and I think a system property is sufficient for a first pass at configuration. It's likely something people will turn on in their build script and forget about. And we really want to encourage people to go all-in for randomization instead of case-by-case.

[ashleyfrieze]

@tjarratt please review #60 which is where the introduction of PreConditions and SpectrumOptions is being played with. I think that randomization may well be a precondition. Whether it defaults to ON or not is another matter. While you're right that you should work out a way forward without too much co-dependence on other PRs, I'm trying to get some fairly hefty changes in at the moment, which may conflict with what you're trying to do. However, I hope that these PRs open up the possibility of adding your capabilities in more easily.

[ashleyfrieze]

@tjarratt the ConfiguredBlock work in #60 eventually merged in, and @greghaskins went further with it in #64. What's now on master would be a good starting point for your randomOrder directive. Do you want to propose a PR? Do you want me to take a look at something?

[ashleyfrieze]

A basic implementation of this, which is not yet tested/ready is here - ashleyfrieze@301e277

Early feedback appreciated. See the Readme first.

[tjarratt]

This looks awesome! Thanks for putting in the hard work, @ashleyfrieze. I had high hopes to contribute here, but my initial efforts fizzled out late last year.

Really looking forward to trying this out soon 😄

[ashleyfrieze]

It will be down to @greghaskins on the DSL side of this. It doesn't quite work the way that was discussed higher-up the thread. In other words, there's no defaults to random order thing. It's just an option you can take if you wish.

Note also the ability to make the random order repeatable to diagnose failures. I think that's probably a wise idea. Not sure how well it would work in practice.

Note also the fact that randomisation is within the hierarchy, not across it. So randomisation happens inside a parent, not across every leaf node.

I took this one on to see how the test runner would react to tests executing in a different order than they are discovered. In IntelliJ it seems fine.