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

fix nested describe.only suites; closes #2406 #2411

Merged

Conversation

not-an-aardvark
Copy link
Contributor

@not-an-aardvark not-an-aardvark commented Aug 3, 2016

Previously, tests in nested describe.only suites would not be registered as only tests in their parent suite (due to the implementation described in #2387 (comment)). This caused mocha to think that the parent suite had no only tests, so the suite was skipped entirely. (See #2406 for more information.)

This PR moves some of the logic for it.only and describe.only to the test runner, rather than the place where suites are created. This makes the logic slightly simpler, because all of the suites already exist at the time that only suites are handled.


The correct logic for only blocks (from what I can tell, looking at the existing tests and the docs) is as follows:

To run a suite:

  • If a suite contains no only suites/tests among all of its descendants, run all of its child suites and tests.
  • Otherwise, if a suite contains any only tests among its children, run the only tests for that suite, and skip all the child suites.
  • Otherwise, run all the child only suites. Also run any child suites that have only suites/tests as descendants.

@boneskull
Copy link
Member

Thanks for this. I'm re-running the appveyor build; it may have been timing flake.

@boneskull
Copy link
Member

At first glance, this looks good. If it passes CI, I'll see if I can't release it as v3.0.1 tonight or tomorrow.

@boneskull
Copy link
Member

@not-an-aardvark Is there anything I should know about with the signed commit? I haven't worked with these before, and don't know the implications (if any).

@not-an-aardvark
Copy link
Contributor Author

not-an-aardvark commented Aug 3, 2016

I don't think there are any implications to worry about; it basically just means you can be sure that the person with the not-an-aardvark GitHub account is the same person who made the commit. (This information would be more useful if another user had a branch with some of my commits on it, for example. In this case it's sort of trivial because I made the PR anyway.)

@boneskull
Copy link
Member

@not-an-aardvark tyvm!

@boneskull boneskull merged commit 8afe661 into mochajs:master Aug 4, 2016
@not-an-aardvark not-an-aardvark deleted the 2406-nested-describe-only branch August 4, 2016 05:05
sgilroy pushed a commit to TwineHealth/mocha that referenced this pull request Feb 27, 2019
…ribe-only

fix nested describe.only suites; closes mochajs#2406
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants