-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
import/require xit, fixes #2972 #2997
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @solodynamo, thanks for submitting this!
I identified one thing that needs to be updated, see the code comment below for details.
In the process of looking into that, I also identified that there are the following x
functions you can feel free to add all of if you'd like:
xit
xspecify
(exactly the same asxit
)xdescribe
xcontext
(exactly the same asxdescribe
)
xdescribe
and xcontext
, like xit
as explained below, can take from context.suite.skip
for the TDD side, given its lack of an x
version. Both the TDD and the BDD suite skip
functions and the BDD suite x
functions are all delegating to the same thing and will therefore be equivalent.
Now that I look at it, it seems we're also missing the BDD's specify
(identical to it
) and context
(context.context
in this code; identical to describe
). You can add those here if you like too.
If you don't want to add all of these that's fine too, I just figured I'd point them out -- and have this comment listing them so they can be addressed at some point -- since I happened to identify them all when comparing how to handle xit
with test.skip
in the ||
branch.
lib/mocha.js
Outdated
@@ -204,6 +204,7 @@ Mocha.prototype.ui = function (name) { | |||
exports.before = context.before || context.suiteSetup; | |||
exports.describe = context.describe || context.suite; | |||
exports.it = context.it || context.test; | |||
exports.xit = context.xit || context.test; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(This is the only change that should be needed to merge this. Any others suggested are optional.)
So, the context.test
on the right side of the ||
isn't right, that's the same as for the non-skipped it
. (With that said, you can skip to the very bottom for recommended fix; the rest here is explanation.) I'm not 100% certain whether this involves the QUnit interface at all, but it seems these are basically enabling require
-based used of either the BDD or the TDD interface when either the BDD or TDD interface is used; so, the ||
here would be to replicate the BDD interface in the event the TDD interface is chosen. So, I double-checked what the TDD interface does for skipping.
In the BDD interface, xit
, xspecify
and it.skip
are all set to a function that just calls it
while only using the title. In the TDD interface, there are no x
functions, and test.skip
delegates to the "common interface" test.skip
, which in turn relies on the TDD interface's test
function and -- like the BDD function -- just passes it only the title. (Weird coupling between the "common interface" and the TDD interface... but nothing worth worrying about here.)
So it looks like it should be perfectly safe to change this to || context.test.skip
.
@@ -1464,6 +1464,7 @@ Mocha.prototype.ui = function (name) { | |||
exports.before = context.before || context.suiteSetup; | |||
exports.describe = context.describe || context.suite; | |||
exports.it = context.it || context.test; | |||
exports.xit = context.xit || context.test; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small tip: you don't actually have to update the mocha.js
file at the base of the project, as it is generated from the other files as part of the publication process. (On the other hand, for the same reason, you don't have to not update it either -- it will simply be overwritten later, as long as you update the other files in addition. I just mention this because it's useful to know you don't have search through the gigantic bundle for the code to update and do have to update other files for changes to actually be picked up.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ScottFreeCode Updated the PR with suggested changes. This time not updated mocha.js at root level.
Squashed commits and merged in 1cc0fc0 (sorry that didn't automatically update this PR); thanks again! |
This Pull Request updates dependency [mocha](https://github.com/mochajs/mocha) from `v3.1.2` to `v3.5.3` <details> <summary>Release Notes</summary> ### [`v3.5.3`](https://github.com/mochajs/mocha/blob/master/CHANGELOG.md#​353--2017-09-11) [Compare Source](mochajs/mocha@v3.5.2...v3.5.3) #### 🐛 Fixes - [#​3003]: Fix invalid entities in xUnit reporter first appearing in v3.5.1 ([@​jkrems]) [#​3003]: `https://github.com/mochajs/mocha/pull/3003` --- ### [`v3.5.2`](https://github.com/mochajs/mocha/blob/master/CHANGELOG.md#​352--2017-09-10) [Compare Source](mochajs/mocha@v3.5.1...v3.5.2) #### 🐛 Fixes - [#​3001]: Fix AMD-related failures first appearing in v3.5.1 ([@​boneskull]) [#​3001]: `https://github.com/mochajs/mocha/pull/3001` --- ### [`v3.5.1`](https://github.com/mochajs/mocha/blob/master/CHANGELOG.md#​351--2017-09-09) [Compare Source](mochajs/mocha@v3.5.0...v3.5.1) #### 📰 News - 📣 Mocha is now sponsoring [PDXNode](http://pdxnode.org)! If you're in the [Portland](https://wikipedia.org/wiki/Portland,_Oregon) area, come check out the monthly talks and hack nights! #### 🐛 Fixes - [#​2997]: Fix missing `xit` export for "require" interface ([@​solodynamo]) - [#​2957]: Fix unicode character handling in XUnit reporter failures ([@​jkrems]) #### 🔩 Other - [#​2986]: Add issue and PR templates ([@​kungapal]) - [#​2918]: Drop bash dependency for glob-related tests ([@​ScottFreeCode]) - [#​2922]: Improve `--compilers` coverage ([@​ScottFreeCode]) - [#​2981]: Fix tpyos and spelling errors ([@​jsoref]) [#​2997]: `https://github.com/mochajs/mocha/pull/2997` [#​2957]: `https://github.com/mochajs/mocha/pull/2957` [#​2918]: `https://github.com/mochajs/mocha/pull/2918` [#​2986]: `https://github.com/mochajs/mocha/pull/2986` [#​2922]: `https://github.com/mochajs/mocha/pull/2922` [#​2981]: `https://github.com/mochajs/mocha/pull/2981` [@​solodynamo]: https://github.com/solodynamo [@​jkrems]: https://github.com/jkrems [@​jsoref]: https://github.com/jsoref --- ### [`v3.5.0`](https://github.com/mochajs/mocha/blob/master/CHANGELOG.md#​350--2017-07-31) [Compare Source](mochajs/mocha@v3.4.2...v3.5.0) #### 📰 News - Mocha now has a [code of conduct](https://github.com/mochajs/mocha/blob/master/.github/CODE_OF_CONDUCT.md) (thanks [@​kungapal]!). - Old issues and PRs are now being marked "stale" by [Probot's "Stale" plugin](https://github.com/probot/stale). If an issue is marked as such, and you would like to see it remain open, simply add a new comment to the ticket or PR. - **WARNING**: Support for non-ES5-compliant environments will be dropped starting with version 4.0.0 of Mocha! #### 🔒 Security Fixes - [#​2860]: Address [CVE-2015-8315](https://nodesecurity.io/advisories/46) via upgrade of [debug](https://npm.im/debug) ([@​boneskull]) #### 🎉 Enhancements - [#​2696]: Add `--forbid-only` and `--forbid-pending` flags. Use these in CI or hooks to ensure tests aren't accidentally being skipped! ([@​charlierudolph]) - [#​2813]: Support Node.js 8's `--napi-modules` flag ([@​jupp0r]) #### 🔩 Other - Various CI-and-test-related fixes and improvements ([@​boneskull], [@​dasilvacontin], [@​PopradiArpad], [@​Munter], [@​ScottFreeCode]) - "Officially" support Node.js 8 ([@​elergy]) [#​2860]: `https://github.com/mochajs/mocha/pull/2860` [#​2696]: `https://github.com/mochajs/mocha/pull/2696` [#​2813]: `https://github.com/mochajs/mocha/pull/2813` [@​charlierudolph]: https://github.com/charlierudolph [@​PopradiArpad]: https://github.com/PopradiArpad [@​kungapal]: https://github.com/kungapal [@​elergy]: https://github.com/elergy [@​jupp0r]: https://github.com/jupp0r --- ### [`v3.4.2`](https://github.com/mochajs/mocha/blob/master/CHANGELOG.md#​342--2017-05-24) [Compare Source](mochajs/mocha@v3.4.1...v3.4.2) #### 🐛 Fixes - [#​2802]: Remove call to deprecated `os.tmpDir` ([@​makepanic]) - [#​2820]: Eagerly set `process.exitCode` ([@​chrisleck]) #### 🔩 Other - [#​2778]: Move linting into an npm script ([@​Munter]) [@​chrisleck]: https://github.com/chrisleck [@​makepanic]: https://github.com/makepanic [@​Munter]: https://github.com/Munter [#​2778]: `https://github.com/mochajs/mocha/pull/2778` [#​2802]: `https://github.com/mochajs/mocha/issues/2802` [#​2820]: `https://github.com/mochajs/mocha/pull/2820` --- ### [`v3.4.1`](https://github.com/mochajs/mocha/blob/master/CHANGELOG.md#​341--2017-05-14) [Compare Source](mochajs/mocha@v3.3.0...v3.4.1) Fixed a publishing mishap with git's autocrlf settings. --- ### [`v3.3.0`](https://github.com/mochajs/mocha/blob/master/CHANGELOG.md#​330--2017-04-24) [Compare Source](mochajs/mocha@v3.2.0...v3.3.0) Thanks to all our contributors, maintainers, sponsors, and users! ❤️ As highlights: - We've got coverage now! - Testing is looking less flaky \o/. - No more nitpicking about "mocha.js" build on PRs. #### 🎉 Enhancements - [#​2659]: Adds support for loading reporter from an absolute or relative path ([@​sul4bh]) - [#​2769]: Support `--inspect-brk` on command-line ([@​igwejk]) #### 🐛 Fixes - [#​2662]: Replace unicode chars w/ hex codes in HTML reporter ([@​rotemdan]) #### 🔍 Coverage - [#​2672]: Add coverage for node tests ([@​c089], [@​Munter]) - [#​2680]: Increase tests coverage for base reporter ([@​epallerols]) - [#​2690]: Increase tests coverage for doc reporter ([@​craigtaub]) - [#​2701]: Increase tests coverage for landing, min, tap and list reporters ([@​craigtaub]) - [#​2691]: Increase tests coverage for spec + dot reporters ([@​craigtaub]) - [#​2698]: Increase tests coverage for xunit reporter ([@​craigtaub]) - [#​2699]: Increase tests coverage for json-stream, markdown and progress reporters ([@​craigtaub]) - [#​2703]: Cover .some() function in utils.js with tests ([@​seppevs]) - [#​2773]: Add tests for loading reporters w/ relative/absolute paths ([@​sul4bh]) #### 🔩 Other - Remove bin/.eslintrc; ensure execs are linted ([@​boneskull]) - [#​2542]: Expand CONTRIBUTING.md ([@​boneskull]) - [#​2660]: Double timeouts on integration tests ([@​Munter]) - [#​2653]: Update copyright year ([@​Scottkao85], [@​Munter]) - [#​2621]: Update dependencies to enable Greenkeeper ([@​boneskull], [@​greenkeeper]) - [#​2625]: Use trusty container in travis-ci; use "artifacts" addon ([@​boneskull]) - [#​2670]: doc(CONTRIBUTING): fix link to org members ([@​coderbyheart]) - Add Mocha propaganda to README.md ([@​boneskull]) - [#​2470]: Avoid test flake in "delay" test ([@​boneskull]) - [#​2675]: Limit browser concurrency on sauce ([@​boneskull]) - [#​2669]: Use temporary test-only build of mocha.js for browsers tests ([@​Munter]) - Fix "projects" link in README.md ([@​boneskull]) - [#​2678]: Chore(Saucelabs): test on IE9, IE10 and IE11 ([@​coderbyheart]) - [#​2648]: Use `semistandard` directly ([@​kt3k]) - [#​2727]: Make the build reproducible ([@​lamby]) [@​boneskull]: https://github.com/boneskull [@​c089]: https://github.com/c089 [@​coderbyheart]: https://github.com/coderbyheart [@​craigtaub]: https://github.com/craigtaub [@​epallerols]: https://github.com/epallerols [@​greenkeeper]: https://github.com/greenkeeper [@​igwejk]: https://github.com/igwejk [@​kt3k]: https://github.com/kt3k [@​lamby]: https://github.com/lamby [@​Munter]: https://github.com/Munter [@​rotemdan]: https://github.com/rotemdan [@​seppevs]: https://github.com/seppevs [@​sul4bh]: https://github.com/sul4bh [#​2470]: `https://github.com/mochajs/mocha/pull/2470` [#​2542]: `https://github.com/mochajs/mocha/issues/2542` [#​2621]: `https://github.com/mochajs/mocha/pull/2621` [#​2625]: `https://github.com/mochajs/mocha/pull/2625` [#​2648]: `https://github.com/mochajs/mocha/pull/2648` [#​2653]: `https://github.com/mochajs/mocha/pull/2653` [#​2659]: `https://github.com/mochajs/mocha/pull/2659` [#​2660]: `https://github.com/mochajs/mocha/pull/2660` [#​2662]: `https://github.com/mochajs/mocha/pull/2662` [#​2669]: `https://github.com/mochajs/mocha/pull/2669` [#​2670]: `https://github.com/mochajs/mocha/pull/2670` [#​2672]: `https://github.com/mochajs/mocha/pull/2672` [#​2675]: `https://github.com/mochajs/mocha/pull/2675` [#​2678]: `https://github.com/mochajs/mocha/pull/2678` [#​2680]: `https://github.com/mochajs/mocha/pull/2680` [#​2690]: `https://github.com/mochajs/mocha/pull/2690` [#​2691]: `https://github.com/mochajs/mocha/pull/2691` [#​2698]: `https://github.com/mochajs/mocha/pull/2698` [#​2699]: `https://github.com/mochajs/mocha/pull/2699` [#​2701]: `https://github.com/mochajs/mocha/pull/2701` [#​2703]: `https://github.com/mochajs/mocha/pull/2703` [#​2727]: `https://github.com/mochajs/mocha/pull/2727` [#​2769]: `https://github.com/mochajs/mocha/pull/2769` [#​2773]: `https://github.com/mochajs/mocha/pull/2773` --- ### [`v3.2.0`](https://github.com/mochajs/mocha/blob/master/CHANGELOG.md#​320--2016-11-24) [Compare Source](mochajs/mocha@v3.1.2...v3.2.0) #### 📰 News ##### Mocha is now a JS Foundation Project! Mocha is proud to have joined the [JS Foundation](https://js.foundation). For more information, [read the announcement](https://js.foundation/announcements/2016/10/17/Linux-Foundation-Unites-JavaScript-Community-Open-Web-Development/). ##### Contributor License Agreement Under the foundation, all contributors to Mocha must sign the [JS Foundation CLA](https://js.foundation/CLA/) before their code can be merged. When sending a PR--if you have not already signed the CLA--a friendly bot will ask you to do so. Mocha remains licensed under the [MIT license](https://github.com/mochajs/mocha/blob/master/LICENSE). #### 🐛 Bug Fix - [#​2535]: Fix crash when `--watch` encounters broken symlinks ([@​villesau]) - [#​2593]: Fix (old) regression; incorrect symbol shown in `list` reporter ([@​Aldaviva]) - [#​2584]: Fix potential error when running XUnit reporter ([@​vobujs]) #### 🎉 Enhancement - [#​2294]: Improve timeout error messaging ([@​jeversmann], [@​boneskull]) - [#​2520]: Add info about `--inspect` flag to CLI help ([@​ughitsaaron]) #### 🔩 Other - [#​2570]: Use [karma-mocha](https://npmjs.com/package/karma-mocha) proper ([@​boneskull]) - Licenses updated to reflect new copyright, add link to license and browser matrix to `README.md` ([@​boneskull], [@​ScottFreeCode], [@​dasilvacontin]) [#​2294]: `https://github.com/mochajs/mocha/issues/2294` [#​2535]: `https://github.com/mochajs/mocha/issues/2535` [#​2520]: `https://github.com/mochajs/mocha/pull/2520` [#​2593]: `https://github.com/mochajs/mocha/pull/2593` [#​2584]: `https://github.com/mochajs/mocha/issues/2584` [#​2570]: `https://github.com/mochajs/mocha/issues/2570` [@​Aldaviva]: https://github.com/Aldaviva [@​jeversmann]: https://github.com/jeversmann [@​ughitsaaron]: https://github.com/ughitsaaron [@​villesau]: https://github.com/villesau [@​vobujs]: https://github.com/vobujs Thanks to all our contributors, sponsors and backers! Keep on the lookout for a public roadmap and new contribution guide coming soon. --- </details> --- This PR has been generated by [Renovate Bot](https://renovatebot.com).
Description of the Change
Adds
xit
into the exports in the fileslib/mocha.js
andmocha.js
.Why should this be in core?
To individually require/import
xit
from the library.Benefits
It will allow devs to skip a particular test. Another alternative is
it.skip