jasmine / jasmine

Simple JavaScript testing framework for browsers and node.js
http://jasmine.github.io/
MIT License
15.75k stars 2.24k forks source link

--fail-fast fails forgetfully (only skips upcoming specs in the current suite, then runs specs in the next suite) #1563

Closed alexch closed 6 years ago

alexch commented 6 years ago

Are you creating an issue in the correct repository?

The jasmine-npm tool has a --fail-fast=true command line option but it seems to be operating correctly (it parses and passes flags); the real issue is apparently inside jasmine-core.

Expected Behavior

When running a spec suite with several describe blocks, if you specify --fail-fast=true, it should abort the entire test run, skipping all upcoming specs and describes, and reporting only a single failure.

Current Behavior

When running a spec suite with several describe blocks, if you specify --fail-fast=true, after a failure, it skips upcoming specs in the current block, but continues to run all upcoming describe blocks. Then if a test fails, it skips all upcoming specs in that describe block, but jumps to the next and resets again.

Basically, given the spec below:

describe("Apple", function() {
    it("mercury", function() {
        fail();
    });
    it("venus", function() {
        fail();
    });
    it("earth", function() {
        fail();
    });
});

describe("Banana", function() {
    it("mars", function() {
        fail();
    });
    it("jupiter", function() {
        fail();
    });
    it("saturn", function() {
        fail();
    });
});

describe("Cherry", function() {
    it("uranus", function() {
        fail();
    });
    it("neptune", function() {
        fail();
    });
    it("pluto", function() {
        fail();
    });
});

If you run it with fail-fast=true and --random=false, you should see only a single failure ("Apple mercury") but instead you see three failures ("Apple mercury", "Banana mars", and "Cherry uranus").

Failures:
1) Apple mercury
  Message:
    Failed
  Stack:
    Error: Failed
        at <Jasmine>
    Error: Failed
        at <Jasmine>
        at UserContext.<anonymous> (/Users/alex/code/fail-fast-forgets.spec.js:3:9)
        at <Jasmine>

2) Banana mars
  Message:
    Failed
  Stack:
    Error: Failed
        at <Jasmine>
        at UserContext.<anonymous> (/Users/alex/code/fail-fast-forgets.spec.js:15:9)
        at <Jasmine>

3) Cherry uranus
  Message:
    Failed
  Stack:
    Error: Failed
        at <Jasmine>
        at UserContext.<anonymous> (/Users/alex/code/fail-fast-forgets.spec.js:27:9)
        at <Jasmine>

Possible Solution

I have implemented a solution but it's a hack...

nodeComplete() sets a variable hasFailures which would seem to do the trick. I'd like to check it inside Spec.execute and if fail-fast is set and hasFailures is true, skip lightly on, like xit does.

But sadly, I couldn't figure out how to get access to hasFailures from inside Spec.execute (jasmine.js:620 or so).

So I wrote a global accessor and got it that way, checking it at the same place pending and excluded specs are checked:

if (this.markedPending || excluded === true || $HACK_HAS_FAILURES()) {

and this provided the correct behavior (only report the first failure).

If someone can point me in the right direction I'll take another stab at it.

Suite that reproduces the behavior (for bugs)

The above suite reproduces the behavior but I haven't written a meta-spec that would assert that if you pass in the proper command line options and suite, that only one failure is reported.

Context

I am teaching a class using test-guided exercises from exercism.io and testfirst.org. In that class it is important that the students work on one feature (spec) at a time, then move on to the next. Seeing a dozen test failures is confusing, especially when they're in random order.

(As a side issue, IMHO specifying fail-fast should also change the default value of random to false, otherwise you would potentially see a different test failing on each run, which negates the use case of fail-fast to focus on a single failure at a time. I may submit this as a feature request or PR later.)

Your Environment

alexch commented 6 years ago

for the record, https://github.com/jasmine/jasmine/commit/e15f273f06c6b8c7902050fe500486c056802ed1 is the commit that added proper fail-fast support and https://github.com/jasmine/jasmine/issues/414 is the issue that requested it -- ditto for jasmine-npm here: https://github.com/jasmine/jasmine-npm/commit/375b7aa92df7043aa5242dac0e1ef24d3284ffcc and here https://github.com/jasmine/jasmine-npm/issues/16