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

dynamic skip -- works in some places... #1625

Closed
shaunc opened this issue Mar 25, 2015 · 10 comments
Closed

dynamic skip -- works in some places... #1625

shaunc opened this issue Mar 25, 2015 · 10 comments
Labels
type: bug a defect, confirmed by a maintainer

Comments

@shaunc
Copy link

shaunc commented Mar 25, 2015

Thanks very much for dynamic skip!

It works well for me like this:

describe "foo", ->
   beforeEach ->
       @skip()
    it "should foo", ->

However, it doesn't work if the "beforeEach is in a wrapping suite:

describe "foo", ->
   beforeEach ->
       @skip()
    describe "bar", ->
        it "should foo", ->

Also doesn't seem to work in "before".

@danielstjules
Copy link
Contributor

Thanks for mentioning it! We've been discussing this bug in #1618 (comment) as well.

@boneskull
Copy link
Contributor

@danielstjules should this be closed?

@danielstjules
Copy link
Contributor

@boneskull no, sorry for the confusion - that's my bad. It's a separate issue from #1604 The PR I brought up gives us the ability to use this.skip() from an async spec/hook. However, the PR also brought up the fact that the existing synchronous skip behavior, on which its based, has a bug.

For another example, see #1618 (comment) The result of that test suite should probably be 2 pending specs.

@shaunc
Copy link
Author

shaunc commented Mar 30, 2015

pace #1618 -- my tests look sync, but they are using promises, so actually async. in "before" I do some db setup, which I need to have done before I can decide on skip(), so wrapping for delayed definition becomes cumbersome. I could put the whole module inside setupDB.then( ... ), but this is not ideal -- dynamic skip is a great feature for me. :)

@danielstjules
Copy link
Contributor

Ah I see - using skip() from async logic isn't supported at the moment, but if #1618 is merged, that might change!

@boneskull boneskull added the type: bug a defect, confirmed by a maintainer label Apr 9, 2015
@dmarcelino
Copy link

+1 for async skip!

dmarcelino added a commit to dmarcelino/it-optional that referenced this issue May 30, 2015
@ScottFreeCode
Copy link
Contributor

Also doesn't seem to work in "before".

Reviewing this issue since some of the other async skip issues have since been fixed; I expanded the test cases to cover all combinations like so:

test.coffee

describe "beforeEach", ->
    describe "outer", ->
        beforeEach ->
            @skip()
        it "should be skipped", ->

    describe "outer", ->
        beforeEach ->
            @skip()
        describe "inner", ->
            it "should be skipped", ->

    describe "outer", ->
        beforeEach ->
            @skip()
        it "should be skipped", ->
        describe "inner", ->
            it "should be skipped", ->

describe "before", ->
    describe "outer", ->
        before ->
            @skip()
        it "should be skipped", ->

    describe "outer", ->
        before ->
            @skip()
        describe "inner", ->
            it "should be skipped", ->

    describe "outer", ->
        before ->
            @skip()
        it "should be skipped", ->
        describe "inner", ->
            it "should be skipped", ->

(And, just to be on the safe side:)

test.js

describe("beforeEach", function() {
    describe("outer", function() {
        beforeEach(function() {
            this.skip()
        })
        it("should be skipped", function() {})
    })

    describe("outer", function() {
        beforeEach(function() {
            this.skip()
        })
        describe("inner", function() {
            it("should be skipped", function() {})
        })
    })

    describe("outer", function() {
        beforeEach(function() {
            this.skip()
        })
        it("should be skipped", function() {})
        describe("inner", function() {
            it("should be skipped", function() {})
        })
    })
})

describe("before", function() {
    describe("outer", function() {
        before(function() {
            this.skip()
        })
        it("should be skipped", function() {})
    })

    describe("outer", function() {
        before(function() {
            this.skip()
        })
        describe("inner", function() {
            it("should be skipped", function() {})
        })
    })

    describe("outer", function() {
        before(function() {
            this.skip()
        })
        it("should be skipped", function() {})
        describe("inner", function() {
            it("should be skipped", function() {})
        })
    })
})

Both output:

 beforeEach
   outer
     - should be skipped
   outer
     inner
       - should be skipped
   outer
     - should be skipped
     inner
       - should be skipped

 before
   outer
     - should be skipped
   outer
     inner
       √ should be skipped
   outer
     - should be skipped
     inner
       √ should be skipped


 2 passing (62ms)
 6 pending

@mochajs/core Is it expected that this.skip in before hooks would not apply to nested suites? I'm not entirely clear whether #2571 covers that.

@stale
Copy link

stale bot commented Oct 17, 2017

I am a bot that watches issues for inactivity.
This issue hasn't had any recent activity, and I'm labeling it stale. In 14 days, if there are no further comments or activity, I will close this issue.
Thanks for contributing to Mocha!

@stale stale bot added the stale this has been inactive for a while... label Oct 17, 2017
@boneskull boneskull removed the stale this has been inactive for a while... label Oct 18, 2017
@boneskull
Copy link
Contributor

let's try to consolidate the open issues around the skip behavior.

@juergba
Copy link
Contributor

juergba commented Jan 24, 2019

 beforeEach
    outer
      - should be skipped
    outer
      inner
        - should be skipped
    outer
      - should be skipped
      inner
        - should be skipped

  before
    outer
      - should be skipped
    outer
      inner
        - should be skipped
    outer
      - should be skipped
      inner
        - should be skipped

  0 passing (19ms)
  8 pending

With current master this.skip applies consistently to nested suites for both before and beforeEach hooks.

@juergba juergba closed this as completed Jan 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug a defect, confirmed by a maintainer
Projects
None yet
Development

No branches or pull requests

6 participants