this.skip() is also skipping teardown()/afterEach()

[laughinghan]

    setup(function() {
      mock.innerHTML = 'Temporary stuff';
    });
    teardown(function() {
      mock.innerHTML = ''; // THIS ISN'T BEING RUN
    });
    test('test', function() {
      if ('some precondition is unmet') {
        this.skip();
      }
    });

With Mocha v2.4.5: http://jsbin.com/bijuru/edit?html,output
With latest master branch: http://jsbin.com/dajugel/edit?html,output

Observed: Temporary stuff is leftover from test setup

Expected: Temporary stuff should've been cleared by test teardown

[danielstjules]

Confirmed as an issue. Likely extends to the bdd and other UIs as well.

suite('suite', function() {
  suiteSetup(function() {
    console.log('SUITE SETUP');
  });

  setup(function() {
    console.log('SETUP');
  });

  test('spec2', function() {
    console.log('SKIPPED SPEC');
    this.skip();
  });

  teardown(function() {
    console.log('TEADOWN');
  });

  suiteTeardown(function() {
    console.log('SUITE TEARDOWN');
  });
});

// =>

SUITE SETUP
SETUP
SKIPPED SPEC
SUITE TEARDOWN

[danielstjules]

Note that the behavior is correct when using test.skip to skip the function. In that instance, the setup/beforeEach is never ran, so the teardown/afterEach is unnecessary.

It's only when dynamically skipping the spec that this occurs. In that case, the test is pending, however the setup/beforeEach has already ran.

[boneskull]

@megagon @danielstjules 8a37e01 was merged. Is this still an issue?

[laughinghan]

@boneskull: yes it is, I'm able to repro using the testcase from the original issue report that uses latest master, you can see "Temporary stuff" isn't cleaned up: http://jsbin.com/dajugel/edit?html,output

[ScottFreeCode]

I copied this down locally and tried it with a rebuilt browser mocha.js file and confirmed the issue is still present. In the process I also noticed how this is different from #2286/#2287: that was afterAll being skipped if beforeEach calls skip, this is afterEach being skipped if the test or the beforeEach calls skip.

[laughinghan]

@ScottFreeCode: oh, thanks! Did you ever try #2168?

[ScottFreeCode]

If I take the change from #2168 and rebuild mocha.js, it does seem to fix this issue. It could use some updates to the tests though.

[boneskull]

dupe of #2286 for all intents and purposes

[boneskull]

See PR #2571

[laughinghan]

Hey now, mine came first so strictly speaking #2286 is a dupe of this one 😜