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

Feature: Add ability to bail from current suite only #2224

Closed
hoverduck opened this Issue Apr 26, 2016 · 4 comments

Comments

Projects
None yet
3 participants
@hoverduck

hoverduck commented Apr 26, 2016

I have a lot of tests that are dependent on previous steps, and when an early step fails it causes everything after it to fail too, leading to a lot of unnecessary noise. There's only so much that makes sense to go in the before() block. I've implemented the feature myself, but the contributions guidelines say not to PR new features, so I'll just link to my fork here: master...hoverduck:master

It's a relatively simple change, but it's possible (likely) I missed something, so I'd love feedback even if you opt not to accept a PR for it.

My implementation works by calling this.bailSuite( true ); at the top of the suite (inside the describe() block) you want chunked together, and then on failure it will bail out of that suite to the next one at the same level in the hierarchy.

@danielstjules

This comment has been minimized.

Contributor

danielstjules commented Apr 27, 2016

Interesting idea! I realize there's def times where it's hard to manage all the dependencies and requirements for a spec. It's generally considered an anti-pattern to have tests depend on the execution of other tests - a good test suite should be able to have its individual specs run in any order. But as you've highlighted, you can end up with a lot of noise. I generally see this as being a more common issue with acceptance tests using phanntomjs/selenium/nightmare/etc.

That said, I'm not sure I really see the benefit of bailSuite. If you want reduced noise, shouldn't you just use bail? Why continue to run the test suite at all, since bailSuite ends up masking the total number of errors in your suite. Just because the first error'ed doesn't mean others in the test suite would have. So the reporting can't be accurate?

I'm thinking this is something that would make for a good plugin though! :)

@hoverduck

This comment has been minimized.

hoverduck commented Apr 27, 2016

I generally see this as being a more common issue with acceptance tests using phanntomjs/selenium/nightmare/etc

Exactly right. We're running end-to-end acceptance tests of our system.

If you want reduced noise, shouldn't you just use bail?

Bail reduces the noise too much. I'd rather run all the later tests to know whether I have more problems. Given the choice between too much noise and not running all the test suites, I'd rather run all the suites.

Why continue to run the test suite at all, since bailSuite ends up masking the total number of errors in your suite

I prefer to think of it as squashing all of the errors into a single report. You could make the argument that this is basically duplicating the functionality of the before() hook, but I think this just gives some extra flexibility.

I'm thinking this is something that would make for a good plugin though! :)

Sounds good to me. Is this (#1457) the current status of the plugin system? What's the release schedule look like for that?

@boneskull

This comment has been minimized.

Member

boneskull commented May 23, 2016

@hoverduck no release schedule yet, but code actually exists.

@hoverduck

This comment has been minimized.

hoverduck commented May 23, 2016

Is there a branch to check out or anything where I could give it a try? If it's stable enough to warrant outside feedback yet.

Regardless, I'm going to go ahead and close this issue as my code won't be merged into the baseline.

@hoverduck hoverduck closed this May 23, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment