Opened 12 years ago

Closed 4 years ago

#1518 enhancement closed fixed (fixed)

trial option to exit on failure

Reported by: Jonathan Lange Owned by: Julian Berman
Priority: normal Milestone:
Component: trial Keywords:
Cc: Jerub, acapnotic, strank, Thijs Triemstra Branch: branches/exitfirst-1518
branch-diff, diff-cov, branch-cov, buildbot
Author: glyph

Description (last modified by Thijs Triemstra)

Add an option to Trial to make it stop the test run on the first error/failure.

py.test uses the option:

    -x, --exitfirst    exit instantly on first error or failed test.

which looks fine to me.

Attachments (2)

exitfirst.patch (11.0 KB) - added by Julian Berman 5 years ago.
exitfirst.2.patch (4.3 KB) - added by Julian Berman 5 years ago.

Download all attachments as: .zip

Change History (20)

comment:1 Changed 12 years ago by Jonathan Lange

Status: newassigned

comment:2 Changed 11 years ago by Jonathan Lange

Priority: lownormal

comment:3 Changed 11 years ago by Jonathan Lange

Status: assignednew

Unassigning. Too many other important tickets.

comment:4 Changed 10 years ago by acapnotic

Cc: acapnotic added

comment:5 Changed 7 years ago by Jean-Paul Calderone

Summary: Exit on failuretrial option to exit on failure

comment:6 Changed 7 years ago by wrstlprmpft

Cc: strank added

comment:7 Changed 7 years ago by Thijs Triemstra

Cc: Thijs Triemstra added
Description: modified (diff)

comment:8 Changed 6 years ago by Jonathan Lange

Owner: Jonathan Lange deleted

Changed 5 years ago by Julian Berman

Attachment: exitfirst.patch added

comment:9 Changed 5 years ago by Julian Berman

Keywords: review added

Up for review.

Left out support for --jobs, since that apparently is non-trivial though I didn't look into it. Filed as #6436.

comment:10 Changed 5 years ago by Glyph

Author: glyph
Branch: branches/exitfirst-1518

(In [38448]) Branching to 'exitfirst-1518'

comment:11 Changed 5 years ago by Glyph

(In [38449]) Commit exitfirst.patch refs #1518

comment:12 Changed 5 years ago by Glyph

Here are some build results.

comment:13 Changed 5 years ago by Glyph

Keywords: review removed
Owner: set to Julian Berman

Thanks for the contribution, Julian.

  1. The man page and the command line documentation really need to explain that the option is mutually exclusive with --jobs.
  2. I don't think that unexpected successes should cause the tests to exit. In fact just a couple of hours ago I was remarking to someone that the SUCCESS message really ought to be yellow rather than red, since it doesn't cause a non-zero exit status (and therefore is not really a "failure").
  3. The blank line after the docstring but before the implementation of each test case looks superfluous to me.
  4. The NEWS file is no longer accurate; the flag isn't called --stop or --stop-failed.
  5. There are some twistedchecker errors. However, the way to fix this, I think, is to simply remove the docstrings for these implementation methods, as was recently suggested to me.

Overall a pretty solid feature addition though; if you address these minor issues I think we can land it. I've noticed your name quite a few times around here recently, and you seem to be following coding standards, testing standards etc pretty closely. Would you like to become a committer?

Thanks again!

comment:14 Changed 5 years ago by Julian Berman

Keywords: review added
Owner: Julian Berman deleted

Hey.

  1. Added.
  2. OK. I don't necessarily like that behavior personally, I use --exitfirst to be "hey there's a bug, go fix it" most of the time, but I see py.test doesn't stop after unexpectedSuccess, and nose also probably doesn't but I can't tell at the moment because apparently it's using a TestResult that doesn't have addUnexpectedSuccess anyhow. So I'll cave :).
  3. OK.
  4. Fixed.
  5. Done

Overall a pretty solid feature addition though; if you address these minor issues I think we can land it. I've noticed your name quite a few times around here recently, and you seem to be following coding standards, testing standards etc pretty closely. Would you like to become a committer?

Sure, thanks :) Though I doubt I'd actually help commit anything anytime soon until I spend some time figuring out / bugging tomprince to teach me how to use git-svn properly -- last I checked I think the Wiki needed some updating there now that things are moving along.

Changed 5 years ago by Julian Berman

Attachment: exitfirst.2.patch added

comment:15 in reply to:  14 Changed 5 years ago by Glyph

Replying to Julian:

Hey.

  1. Added.
  2. OK. I don't necessarily like that behavior personally, I use --exitfirst to be "hey there's a bug, go fix it" most of the time, but I see py.test doesn't stop after unexpectedSuccess, and nose also probably doesn't but I can't tell at the moment because apparently it's using a TestResult that doesn't have addUnexpectedSuccess anyhow. So I'll cave :).
  3. OK.
  4. Fixed.
  5. Done

Thanks, I hope someone can take a look at this soon.

Overall a pretty solid feature addition though; if you address these minor issues I think we can land it. I've noticed your name quite a few times around here recently, and you seem to be following coding standards, testing standards etc pretty closely. Would you like to become a committer?

Sure, thanks :) Though I doubt I'd actually help commit anything anytime soon until I spend some time figuring out / bugging tomprince to teach me how to use git-svn properly -- last I checked I think the Wiki needed some updating there now that things are moving along.

The git mirror documentation page is actually almost complete. I can help you out with specifics if you have trouble.

comment:16 Changed 4 years ago by Tom Prince

(In [38690]) Apply exitfirst.2.patch from Julian.

Refs: #1518

comment:17 Changed 4 years ago by Tom Prince

Keywords: review removed
Owner: set to Julian Berman
  1. The man page and the command line documentation really need to explain that the option is mutually exclusive with --jobs.

Added

The manpage has been updated, but the command-line help hasn't.

Other than that, this looks good. Please (get commit access and) merge.

comment:18 Changed 4 years ago by julian

Resolution: fixed
Status: newclosed

(In [38805]) Merge exitfirst-1518: Add --exitfirst to trial

Author: Julian Reviewers: glyph, tomprince Fixes: #1518

Added the --exitfirst flag, allowing one to stop a trial run after the first non-successful test.

Note: See TracTickets for help on using tickets.