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

Option to re-run failing examples #456

Closed
mcmire opened this issue Sep 14, 2011 · 40 comments
Closed

Option to re-run failing examples #456

mcmire opened this issue Sep 14, 2011 · 40 comments

Comments

@mcmire
Copy link
Contributor

mcmire commented Sep 14, 2011

I'm not sure why no one has suggested this yet, but it would be nice if there were a way to do this. This comes in handy especially when you are running a file that has, say, 100 examples and 20 of them fail and you don't want to waste time re-running the other 80. Additionally, whatever is keeping track of failed examples should do so across multiple files that you may be running. So if I am running the full spec suite and some of them fail and they are in separate files, I want the ability to re-run just those.

So the way it'd work is that rspec would write the failed examples to a (non-anonymous) file (this could be a YAML file where the key is a filename and value is an array of line numbers). Given this file contains failed examples, if I then ran rspec --failed, RSpec should read the file and run only those examples, and if any succeed it removes them from the YAML file. If there are no examples left in the YAML file, then rspec --failed runs everything. So obviously running rspec with the option never overwrites the YAML file, but without the option rspec should always overwrite it, that way if you were to run it with the option right afterward you'd get a fresh file.

@justinko
Copy link
Contributor

In my experience, when a suite fails, it is because of only a handful (usually one) of reasons. What that means is if you have 30 failing specs, chances are good that it is caused by one problem. So for me, I'll get the file and line number for one of those failing specs, make it pass, and then chances are very good that the other specs will pass. If that is not the case, that tells me the program has bad encapsulation.

Conclusion: the number of failed examples is not proportionate to the number of things you need to change/fix.

@myronmarston - has this been your experience as well?

@myronmarston
Copy link
Member

That's often the case, but I still think this idea has merit. It sounds a lot like cucumber's rerun option. It would probably be good to align the API with cucumber's API for this.

@mcmire
Copy link
Contributor Author

mcmire commented Sep 15, 2011

I would also agree that is generally true for unit tests, although I also have Capybara integration tests in my Rails app at work (which obviously take a lot longer to run than unit tests) for which that is less often true and for which this would probably be more useful.

I will see if I can take a look at how Cucumber does this and come up with a patch.

@dchelimsky
Copy link
Contributor

We actually had this in rspec-1. My personal experience was that it worked the way I wanted it to most of the time, but when it didn't it took me a while to remember why. If we add this to rspec-2 I'd want the output to indicate that the specs being run were being constrained by the previous failures, and explain how to get around it.

@dcadenas
Copy link

+1

@oggy
Copy link

oggy commented Dec 27, 2011

I'd love this too. In fact, just a built-in formatter which dumped the failing examples out to a file (like rspec 1.x) would make me real happy, as I tend to run rspec through my own wrapper anyway.

If such a formatter would be welcome I'd happily submit a patch. Otherwise I guess I'll do a gem.

@dchelimsky
Copy link
Contributor

It's not enough to have a formatter if we're going to include it in rspec - we also need a means of running just the previously failed examples. I'd prefer this to be a command line option. If you want to submit a patch with both (formatter and command line option to run failed examples) I'd definitely consider it.

@nilclass
Copy link

nilclass commented Jan 4, 2012

This is how it works without a patch (I put it in my spec_helper.rb, in a rails project, but should work without rails as well, with a small adjustment to the first line):

    failure_log_path = Rails.root.join('rerun.txt')

    failed_examples = []

    if File.exist?(failure_log_path)
      rerunning = true
      failed_descriptions = File.readlines(failure_log_path).map(&:chomp)
      config.inclusion_filter = {
        :full_description => ->(desc) {
          failed_descriptions.any? { |d| desc =~ /#{d}/ }
        }
      }
    end

    config.after(:each) do |group|
      if group.example.instance_variable_get("@exception")
        failed_examples.push(group.example)
      end
    end

    config.after(:suite) do
      if failed_examples.any?
        File.open(failure_log_path, 'w') { |f|
          failed_examples.each do |example|
            f.puts(example.metadata[:full_description])
          end
        }
      elsif rerunning
        # no failures -> we (probably) fixed it!
        File.unlink(failure_log_path)
      end
    end

@oggy
Copy link

oggy commented Jan 8, 2012

I'm happy to hear this. :)

I'm going to start on a patch. Here's what I'm thinking:

Simplest usage:

rspec --format=f|failures  # record the failures to the default path
rspec --failed             # rerun all recorded failures

The current directory is recorded along with the failing examples. If this directory no longer matches, you get a warning immediately, but the test run proceeds. This is to reduce confusion when you switch between multiple projects. If you've simply moved the project, however, you can just ignore the warning.

You can record failures to a project-specific file with a --failure-file=PATH option.

You can also specify the exact specs to rerun by passing --failed the numbers of the failures: --failed=1-5,7,13

I find the latter functionality useful when I need to insert a debugger at a point that many specs hit, but only in the context of a certain failing spec.

Thoughts?

@dchelimsky, would this eliminate the confusion you mentioned with rspec-1? Would you still want more than a single line of feedback that basically acknowledges you're in --failed mode?

@nilclass
Copy link

nilclass commented Jan 9, 2012

@oggy, does that mean that while recording failures, the output format (for stdout) cannot be specified any longer? In that case I would suggest going another way and maybe implementing a custom option (--record-failed or something) instead. While recording failures does produce output, it is not strictly a output format in the sense of "documentation", "progress" etc. which are meant to be a representation of results for users, not rspec itself. Any thoughts?

@oggy
Copy link

oggy commented Jan 9, 2012

@nilclass: No. You can already specify multiple -f/--format options. And you can give each one their own -o/--out option.

For this reason, I'm actually inclined to use --out rather than --failure-file when writing the file for consistency with the other formatters. We'd still use --failure-file to specify a custom input failure file - i.e., when rerunning failed examples.

@mcmire
Copy link
Contributor Author

mcmire commented Jan 10, 2012

@nilclass @oggy Good thoughts but why not always record failed examples all the time, why require a --format / --record-failed option? Here's my use case: oftentimes when I want to re-run failed examples, I have already run, say, a folder of examples and only after I do so do I realize I want to re-run the failed ones. If failed examples are always recorded, well hey look at that all I have to do is run rspec --failed and I've done just that, there's no need to go through the trouble of re-running all of them, including the ones I know are successful, when all I really want to do is re-run the ones that aren't. I think it's a more friendlier design to anticipate this. WDYT?

@oggy
Copy link

oggy commented Jan 11, 2012

@mcmire, I dig that. Recording failures should be opt-out-able (e.g. you mightn't have write access to the default location), but otherwise I agree.

@dchelimsky
Copy link
Contributor

I'd consider this, but the question then is where would the file live by convention.

One thing I've thought of is allowing for .rspec to become a directory. What is now .rspec would become .rspec/config and then we could have .rspec/failure_report.txt (or .yml). That would reduce the pollution of the project root and open the door for other file storage, like historical stats (which could be used for ordering as well - run the examples that fail most often first), etc. Obviously it would have to respect a .rspec file as well for compatibility with existing suites, but that wouldn't be difficult to do. It would just mean you couldn't use this feature until you changed the .rspec file to .rspec/config.

Thoughts?

/cc @myronmarston @justinko @patmaddox @spicycode

@nilclass
Copy link

@dchelimsky .rspec.d would do as well and be backwards compatible

@mcmire
Copy link
Contributor Author

mcmire commented Jan 11, 2012

@dchelimsky An .rspec directory makes sense to me.

@oggy
Copy link

oggy commented Jan 11, 2012

I like the .rspec.d idea.

@dchelimsky
Copy link
Contributor

I don't like the .rspec.d idea. There is no compatibility problem here because rspec is the only consumer of .rspec, and we can easily see if there's a file or a directory before proceeding.

@oggy
Copy link

oggy commented Jan 11, 2012

I guess the ".d" suffix doesn't phase me because I've seen the convention used elsewhere often enough.

If we go with an .rspec directory, how about we automatically move any existing .rspec to .rspec/config ?

@dchelimsky
Copy link
Contributor

@oggy - it's not a matter of the .d suffix phasing people - it's a matter of the fact that we already have a convention of .rspec, and there is no need to support both an .rspec file and an .rspec.d/config in the same project. In fact, allowing for either of .rspec (file) and .rspec/config seems less confusing and error prone.

@dchelimsky
Copy link
Contributor

@oggy re: auto-moving .rspec, I wouldn't want that to happen implicitly, but maybe when rspec runs and sees a .rspec file it can print a message to the console saying that it's moving it and wait for a user response. One problem with that approach is that people can't easily downgrade back to the previous release.

I think I'm more inclined to not do this automatically for 2.x, and just document that the failure report will only work if you move .rspec to .rspec/config. Then we can automate it in a 3.0 release. WDYT?

@nilclass
Copy link

@dchelimsky the problem I see with making .rspec a directory is, that older rspec versions will try to open it as a regular file and fail with "Is a directory", which is less likely to point users to the need to upgrade than for example an option in .rspec which isn't recognized.

@oggy
Copy link

oggy commented Jan 13, 2012

After more thought, I'm leaning towards a simpler option: leave the config paths alone until 3.0, and just write failures to .rspec_failures for now. It's not like we're adding a ton of extra files here, and changing the config path seems like a big enough deal for a major version bump. I'd be okay with changing .rspec to a directory on a major version bump.

If we don't go with that option, then my personal preference would still be to use .rspec.d, and just issue a deprecation warning if .rspec exists. I think it's better to have a smooth upgrade/downgrade path on a minor version bump than to maintain a "convention" of using .rspec (I'm not really sure what value that convention is conveying TBH).

Of course I'll code up whatever we settle on here. Just my couple of cents. :)

@nilclass
Copy link

@oggy: sounds like the least troubling solution to me. Have you started with the patch yet? Feel free to push WIP, I'd love to contribute, but right now I'm not deep enough inside rspec code to start this myself. :)

@oggy
Copy link

oggy commented Jan 21, 2012

@nilclass: Just pushed some WIP to my fork in the retry-failures branch.

I've yet to try it out for real, but the one change I plan to make is to have the output file default to the --failure-file setting if it's set. It seems silly to have --failure-file and --out for the failures formatter to be completely independent.

Comments welcome, on this or anything else I've done.

@antifun
Copy link

antifun commented Feb 22, 2012

@oggy, I've been using the retry-failures branch for a while and it's doing exactly what I needed it to do. Have you and @dchelimsky figured out what you want to do about always recording failures? The explicit opt-in is at least a good start and would probably be useful to many people as-is.

@oggy
Copy link

oggy commented Feb 27, 2012

@antifun I haven't received any other feedback. @dchelimsky, do you have any thoughts?

@dchelimsky
Copy link
Contributor

@oggy - I grabbed your branch and gave it a whirl. Overall it's a good start. Here are some thoughts:

  • I'd prefer to declare the file using --out or --format failures:path/to/file over an additional CLI option
  • When there were no failures on the last run, --retry should effectively be ignored. This would allow you to put --retry in .rspec to pre-define your workflow.
  • --retry should also be ignored if there are files and/or specific examples declared on the command line (again, so it could be included in .rspec)

WDYT?

/cc @myronmarston @justinko @patmaddox @spicycode

@nilclass
Copy link

@dchelimsky I don't agree with the last point (ignore --retry if there are files given on command line). Suppose I have a lot of examples (or a few, that take a long time) in a single file, then I still want to be able to run only those that failed without having to specify them. Maybe there should be two options (e.g. --retry and --autoretry) of which one works the way you described and can be put in .rspec, while the other forces retry, even if there are restrictions specified on the command line.

@dchelimsky
Copy link
Contributor

@nilclass I hear you, but this is already more complicated than makes me comfortable. There's a formatter, two new command line options, new files, and new code in 6 pre-existing files. Not sure I want to add so much to support a feature which may or may not be used by the majority of users. I, personally, don't know if I'd use it because I already get similar functionality from tools like autotest and guard.

What would rspec need to support for you to release this as an extension gem? We already have support for plugging in formatters. We'd need a means of plugging in command line options. Anything else?

@antifun
Copy link

antifun commented Feb 29, 2012

I think the only things that are really required here are

  • a formatter that outputs only failed examples
  • an option (doesn't have to be called "retry") that limits rspec to only running specified examples

And of course there's the super-nice-to-have property that rspec emits and consumes the same format in each of those cases.

I don't see the fundamental collision between the "only consider these files" and "only consider these examples" needs; you might require one or the other or both depending on your testing strategy. The "retry" mode is strongly aligned with the latter, but there could be other uses. Would we feel better if it weren't so oriented towards "retry" mode and instead was more simply a way to specify which of the available examples would run?

@dchelimsky
Copy link
Contributor

Again, while the formatter is the means, it doesn't have to be the end. IIRC, rspec-1 always wrote out failures to a file and would then read that file when you had a flag set. We can still use a formatter to do that, but the user doesn't need to know about that formatter. Instead of yml (@oggy's current implementation), the formatter could use the same format we use on the command line to specify examples at specific locations.

./path/to/foo_spec.rb:37
./path/to/bar_spec.rb:42

Now we'd only need one CLI option, something like --[no-]constrain-to-failed-examples (shorter names that tell that same story are welcome). With this flag (without --no), rspec would load the .rspec.failures (or whatever) file and append its contents to the command. You type rspec -F and the resulting command is rspec ./path/to/foo_spec.rb:37 ./path/to/bar_spec.rb:42.

This also has me thinking we can do this in a more general way by supporting a new --append-contents-to-command (probably not really the name, but you get the idea) option that takes a path as an argument and appends the contents of that file to the command before executing it. If we did that, then you could get this functionality by releasing the failures-only formatter as a separate gem (contrary to my statements earlier this comment) and instructing the user to add the following to .rspec:

--format progress
--format FailuresFormatter
--out .failures
--append-contents-to-command .failures

Thoughts?

@antifun
Copy link

antifun commented Feb 29, 2012

I think we are circling the same drain, so to speak, with the last version of your idea. Basically, as long as you can get some formatter to output only the command-line text required to get a particular example to run, and rspec can easily accept that output somehow, then the problem is solved, and people who want to use the "append-contents-to-command" (agree that that is kind of awkward) for other purposes can do so. Whether or not you want to do that "-l style" (with line numbers) or "-e style" would be up to the formatter's choice.

It's really not that far from being possible out of the box, honestly. If you could feed rspec a "failures file" that looked like

-e "Run any test matching this"
-e "Run any test matching that too"
etc.

or

-l ./path/to/foo_spec:45
-l ./path/to/bar_spec:67
etc.

either presumably generated by a formatter that only outputs a line (of one flavor or the other) when an example fails, then use an option like --append-contents-to-command -- in my mind, a great re-use for the -F short option which you'd no longer need -- I think that would do the trick. How does that sound?

@oggy
Copy link

oggy commented Mar 6, 2012

A few thoughts:

  • I agree you can get pretty close as it is right now. As I alluded to in my first comment in this issue, I'd find it handy if the formatter was already there, as it means you simply need a wrapper to pass the right arguments to rspec. In retrospect though, it's not too bad as is - I can just wrap both the formatter and wrapper script in a gem.
  • IME, rerunning examples by name is better than line number - line numbers change much more often than spec names between runs.
  • The -e option in RSpec 2.8.0 is not additive - if you use -e twice, the second overrides the first. If this were changed (or a new additive option added), I'd be happy keeping the rerunning stuff as a separate gem.

@antifun
Copy link

antifun commented Apr 6, 2012

See #596 for my attempt at this.

@dblock
Copy link
Contributor

dblock commented Aug 14, 2012

There's now a gem called rspec-rerun. To use, add it to Gemfile and change the default spec task.

require 'rspec-rerun'
task :default => "rspec-rerun:spec"

@flipkick
Copy link

rspec-retry is another option ( https://github.com/y310/rspec-retry ).

@aprescott
Copy link

I realise this is an old issue but I came here via Google and thought it would be useful for others.

If you want a very simple formatter that will print a single rspec command for re-running all failed examples (up to 10 failing files), check out https://gist.github.com/aprescott/8738673. Has the benefit that it's unobtrusive (minimal, no file creation) and easy to grab from Travis build output, etc.

@Nakilon
Copy link

Nakilon commented Dec 7, 2015

So what's the current preferred solution?

@rspec rspec locked and limited conversation to collaborators Dec 7, 2015
@JonRowe
Copy link
Member

JonRowe commented Dec 7, 2015

RSpec introduced a --only-failures flag in 3.3.0, see the documentation here: http://www.relishapp.com/rspec/rspec-core/v/3-4/docs/command-line/only-failures

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests