Customized ordering strategy for running specs

[owenthereal]

I noticed that spec can be run in random order (https://github.com/rspec/rspec-core/blob/master/lib/rspec/core/extensions/ordered.rb#L12-13). Would it make sense to provide an extension point to run spec in customized order?

Here are some sudo codes:

# spec_helper.rb
RSpec.configure do |config|
  config.order_strategy = lambda {
    ...
  }
end

module RSpec
  module Core
    # @private
    module Extensions
      # @private
      # Used to extend lists of examples and groups to support ordering
      # strategies like randomization.
      module Ordered
        # @private
        def ordered
          if RSpec.configuration.randomize?
            Kernel.srand RSpec.configuration.seed
            sort_by { Kernel.rand size }
          elsif !RSpec.configuration.order_strategy.nil?
            sort_by { RSpec.configuration.order_stategy }
          else
            self
          end
        end
      end
    end
  end
end

It would be nice if I could define what order my specs are run to maximize the feedback loop (e.g., more important spec runs first)

[justinko]

We originally thought of making an extension point for this, but I came to the conclusion that users needing to order in ways other than alpha and random would be few and far between.

In your case, you can achieve what you want with a rake task and metadata. Tag the examples you want to run first with :important and use filtering:

namespace :spec do
  desc "Run important examples"
  RSpec::Core::RakeTask.new(:important) do |t|
    t.rspec_opts = %w[--tag important]
  end

  desc "Run unimportant examples"
  RSpec::Core::RakeTask.new(:unimportant) do |t|
    t.rspec_opts = %w[--tag ~important]
  end
end

task default: ['spec:important', 'spec:unimportant']

@dchelimsky and @myronmarston might have different opinions on this :)

[owenthereal]

Thanks for the reply, @justinko. While I like your approach with explicitly tagging what's important what's not, for those are important/unimportant, they are still running in alphabetical/random order, right? It would be nice any of the examples in the group could be sorted in important order.

Besides, in practice, once a example is tagged, people are too lazy to update the tags (at least in my work environment :-)). Hence lots of tags are deprecated... That's what prompts me to write such strategy class to order specs automatically. I also wrote a filtering strategy to filter what's important what's not based on the history of running specs which is already supported pretty well by rspec. The ordering strategy is the missing part.

I am pretty sure you guys have given a lot of thoughts on what extension points to expect. In case there won't be such extension point, I am thinking of patching rspec like this:

RSpec.configure do |config|
  # all ExampleGroup will be patched
  config.include OrderStrategy
end

module OrderStrategy
  def self.children
    super.sort_by { ... }
  end
end

Would you guys recommend doing something like this?

[myronmarston]

I'd be in favor of a patch like this. I like the idea of letting end users provide custom ordering strategies. I don't think it would add much complexity to support this.

As far as the API, my preference would be something like:

RSpec.configure do |config|
  config.order_examples do |examples|
    # return the examples in the order you want them run
  end
end

I think it's more flexible to let the user order the examples however they want, rather than forcing them to provide a block that returns what to sort by. For example, it makes simple things like this possible:

config.order_examples do |examples|
  important_examples, other_examples = examples.partition { |e| e.tagged_with?(:important) }
  important_examples + other_examples
end

It'd be far more difficult to do things like this if the block had to return a sortable value.

The one main thing I'm not sure of in this scheme is what the config option should be; order_examples is the best I can think of at the moment, but the block would get invoked with example groups and with examples...

[justinko]

The issue I see is what they can order by. To differentiate between examples/groups, there are only a few things you can use: metadata, description, top level description, file name/path.

Even in @myronmarston's example, he's using tagged_with?. Can you guys think of other scenarios where you would not use metadata? I'm not sure how you would use "description" for ordering...because it would essentially be like tagging.

@jingweno - In your case, how would you determine what is important?

[owenthereal]

@justinko I am determining what's important by storing the history of specs running. The more times it fails, the more important it becomes. Does it make sense?

[justinko]

@myronmarston - because of before(:all), we can only pass the example groups, not a flat list of examples. This would bring back your OrderUnit suggestion and a major refactoring...

@jingweno so you would probably have to match against the "full" description of each example. By "full", I mean the example description plus its parent/s description/s. That would be the only way to ensure the match criteria is unique (and not brittle).

[dchelimsky]

@justinko it could also be file + line number, which could be managed in part by @myronmarston's suggestion. Something like:

config.order_examples do |examples|
  locations = File.read('failed_example_locations.txt')
  important_examples, other_examples = examples.partition { |e| locations.include? e.location }
  important_examples + other_examples
end

Not sure what to do about the before(:all) issue, though.

[justinko]

@dchelimsky I would think line number would be brittle though...

Not sure what to do about the before(:all) issue, though.

Yeah, with this issue a lot of users, if not all, will think it's useless (to only be able to order by group, essentially). Technically, we could just pass the examples and raise an error if their suite contains any before(:all)'s. There are a lot of suites that don't have any before(:all) hooks.

Just passing examples wouldn't be too bad of a refactoring I believe.

[dchelimsky]

We could also pass in the groups instead. Then if you want examples you'd just say:

config.order do |groups|
  examples = groups.map {|g| g.examples }.flatten
  # ...
end

[justinko]

@dchelimsky much better and much less refactoring!

[myronmarston]

I hadn't a slightly different solution in mind for the before(:all) issue: keep the running structure as it is now (i.e. example groups are all run together as a unit), but let the ordering block apply both to groups and examples. Thinking about this some more, here's what I think the API should be:

RSpec.configure do |config|
  config.order_groups do |groups|
    # return a list of example groups in order
  end

  config.order_examples do |examples|
    # return a list of examples in order
  end
end

Note that each of these could be called multiple times; i.e. if a group has multiple nested groups, it would invoke the order_groups block with its list; and for each example group with examples, it would invoke the order_examples block with its list.

I think there's value in being able to have separate ordering strategies for the groups vs. examples; for @jingweno's case, he may want to order groups by the number of contained "important" examples (i.e. groups with more important examples get sorted before groups with fewer), and then order the examples within those groups by important vs. not.

On top of that, there are many times when you don't need separate ordering strategies; so we could support that as well:

RSpec.configure do |config|
  config.order_groups_and_examples do |examples_or_groups|
    # return an ordered list
  end
end

Under the covers, this can be implemented very simply:

def order_groups_and_examples(&block)
  order_groups(&block)
  order_examples(&block)
end

In addition, once we have this in place, it's easy to refactor the existing randomization to simply use this API.

Thoughts?

[justinko]

@myronmarston I honestly can't find any gripes with your proposal. I think it could shimmy in just nicely. Good stuff!

[myronmarston]

Hmm...I just realized that my proposal does open up additional flexibility I'm not sure that we want to expose:

RSpec.configure do |config|
  config.order_examples do |examples|
    [RSpec::Core::Example.new()...]
  end
end

While the purpose of the order_examples block is to let people re-order them however, this API allows people to do a lot more than that. Users could filter examples in here, or simply return a static list of examples.

I'm OK with letting people shoot themselves in the foot (this is ruby after all...). @dchelimsky & @justinko -- you guys fine with this?

Regardless, I'll take a stab at implementing this. It'll be good to get back into contributing to RSpec again :).

[dchelimsky]

Fine with me. I doubt anybody would do that by accident :)