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

Use JS for globbing integration test instead of bash shell #2918

Merged
merged 3 commits into from
Aug 2, 2017

Conversation

ScottFreeCode
Copy link
Contributor

Closes #2805

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 89.017% when pulling 91f703e on no-shell-test into 958fbb4 on master.

Copy link
Contributor

@Munter Munter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely done!

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 89.017% when pulling 26d337a on no-shell-test into 82d879f on master.

@ScottFreeCode
Copy link
Contributor Author

I've been able to confirm on my Windows box that Node 0.10.x inconsistently fails the error-only glob tests due to getting an empty string for stderr. I am going to see if I can find a reason why this happens in order to cook up a proper fix; but if I can't find anything I may just make those tests have a retry count when run under Node 0.10 on Windows.

@ScottFreeCode
Copy link
Contributor Author

Noted that when Mocha doesn't find any files it calls process.exit(1).

Tried calling child_process.exec on an exhaustive combination of:

  • node bin/mocha -R json-stream "./**/*-none.js" (aka same as the integration test)
  • same but _mocha instead of mocha (to confirm it's not just the mocha wrapper around _mocha)
  • with --no-exit added (but this probably did nothing, see above)
  • a script that just prints the same warning message to stderr and calls process.exit(1)
  • same but with the process.on("exit", function() { process.exit(1) }) logic from --no-exit (the exitLater function to be precise)
  • same but with the draining logic from Mocha's usual exit path
  • same but with the draining logic except calling the exitLater function instead of process.exit at the end
  • same but with process.on("exit" calling the draining function instead of process.exit
  • same but without calling anything to exit (just letting the script end after printing the message)

In all cases, if I ran this 15 times, eventually it would get an empty string for stderr on Node 0.10 on Windows.

That tells me for sure it is a Node issue and not anything we're doing wrong, since no strategy for exiting can prevent the problem from happening and it's reproducible in a trivial script too; so I am going to brute-force ignore it -- but with a slight twist... rather than just setting the tests to retry, I am going to make the exec-helper function that's already in there robust against getting an empty string for stderr on Node 0.10 on Windows (have it recursively call itself again when that happens). I've even confirmed this workaround is correct by running the test locally a few times and seeing that occasionally the tests that used to sometimes fail instead sometimes take twice as long (implying the integration test call was run twice).

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 89.017% when pulling 8710438 on no-shell-test into 82d879f on master.

@ScottFreeCode
Copy link
Contributor Author

For the record, here are the passing tests:

Going to merge this such that the original commits are retained, in case anyone wants to examine the different steps (conversion from makefile+shell to integration JS, addition of more complete globbing combinations to the test, and finally addition of Node 0.10 Windows workaround).

@ScottFreeCode ScottFreeCode merged commit 075bd51 into master Aug 2, 2017
@ScottFreeCode ScottFreeCode deleted the no-shell-test branch September 14, 2017 05:21
jimthedev referenced this pull request in commitizen/cz-cli May 24, 2018
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#&#8203;353--2017-09-11)
[Compare Source](mochajs/mocha@v3.5.2...v3.5.3)
#### 🐛 Fixes

- [#&#8203;3003]: Fix invalid entities in xUnit reporter first appearing in v3.5.1 ([@&#8203;jkrems])

[#&#8203;3003]: `https://github.com/mochajs/mocha/pull/3003`

---

### [`v3.5.2`](https://github.com/mochajs/mocha/blob/master/CHANGELOG.md#&#8203;352--2017-09-10)
[Compare Source](mochajs/mocha@v3.5.1...v3.5.2)
#### 🐛 Fixes

- [#&#8203;3001]: Fix AMD-related failures first appearing in v3.5.1 ([@&#8203;boneskull])

[#&#8203;3001]: `https://github.com/mochajs/mocha/pull/3001`

---

### [`v3.5.1`](https://github.com/mochajs/mocha/blob/master/CHANGELOG.md#&#8203;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

- [#&#8203;2997]: Fix missing `xit` export for "require" interface ([@&#8203;solodynamo])
- [#&#8203;2957]: Fix unicode character handling in XUnit reporter failures ([@&#8203;jkrems])
#### 🔩 Other

- [#&#8203;2986]: Add issue and PR templates ([@&#8203;kungapal])
- [#&#8203;2918]: Drop bash dependency for glob-related tests ([@&#8203;ScottFreeCode])
- [#&#8203;2922]: Improve `--compilers` coverage ([@&#8203;ScottFreeCode])
- [#&#8203;2981]: Fix tpyos and spelling errors ([@&#8203;jsoref])

[#&#8203;2997]: `https://github.com/mochajs/mocha/pull/2997`
[#&#8203;2957]: `https://github.com/mochajs/mocha/pull/2957`
[#&#8203;2918]: `https://github.com/mochajs/mocha/pull/2918`
[#&#8203;2986]: `https://github.com/mochajs/mocha/pull/2986`
[#&#8203;2922]: `https://github.com/mochajs/mocha/pull/2922`
[#&#8203;2981]: `https://github.com/mochajs/mocha/pull/2981`
[@&#8203;solodynamo]: https://github.com/solodynamo
[@&#8203;jkrems]: https://github.com/jkrems
[@&#8203;jsoref]: https://github.com/jsoref

---

### [`v3.5.0`](https://github.com/mochajs/mocha/blob/master/CHANGELOG.md#&#8203;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 [@&#8203;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

- [#&#8203;2860]: Address [CVE-2015-8315](https://nodesecurity.io/advisories/46) via upgrade of [debug](https://npm.im/debug) ([@&#8203;boneskull])
#### 🎉 Enhancements

- [#&#8203;2696]: Add `--forbid-only` and `--forbid-pending` flags.  Use these in CI or hooks to ensure tests aren't accidentally being skipped! ([@&#8203;charlierudolph])
- [#&#8203;2813]: Support Node.js 8's `--napi-modules` flag ([@&#8203;jupp0r])
#### 🔩 Other

- Various CI-and-test-related fixes and improvements ([@&#8203;boneskull], [@&#8203;dasilvacontin], [@&#8203;PopradiArpad], [@&#8203;Munter], [@&#8203;ScottFreeCode])
- "Officially" support Node.js 8 ([@&#8203;elergy])

[#&#8203;2860]: `https://github.com/mochajs/mocha/pull/2860`
[#&#8203;2696]: `https://github.com/mochajs/mocha/pull/2696`
[#&#8203;2813]: `https://github.com/mochajs/mocha/pull/2813`
[@&#8203;charlierudolph]: https://github.com/charlierudolph
[@&#8203;PopradiArpad]: https://github.com/PopradiArpad
[@&#8203;kungapal]: https://github.com/kungapal
[@&#8203;elergy]: https://github.com/elergy
[@&#8203;jupp0r]: https://github.com/jupp0r

---

### [`v3.4.2`](https://github.com/mochajs/mocha/blob/master/CHANGELOG.md#&#8203;342--2017-05-24)
[Compare Source](mochajs/mocha@v3.4.1...v3.4.2)
#### 🐛 Fixes

- [#&#8203;2802]: Remove call to deprecated `os.tmpDir` ([@&#8203;makepanic])
- [#&#8203;2820]: Eagerly set `process.exitCode` ([@&#8203;chrisleck])
#### 🔩 Other

- [#&#8203;2778]: Move linting into an npm script ([@&#8203;Munter])

[@&#8203;chrisleck]: https://github.com/chrisleck
[@&#8203;makepanic]: https://github.com/makepanic
[@&#8203;Munter]: https://github.com/Munter

[#&#8203;2778]: `https://github.com/mochajs/mocha/pull/2778`
[#&#8203;2802]: `https://github.com/mochajs/mocha/issues/2802`
[#&#8203;2820]: `https://github.com/mochajs/mocha/pull/2820`

---

### [`v3.4.1`](https://github.com/mochajs/mocha/blob/master/CHANGELOG.md#&#8203;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#&#8203;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

- [#&#8203;2659]: Adds support for loading reporter from an absolute or relative path ([@&#8203;sul4bh])
- [#&#8203;2769]: Support `--inspect-brk` on command-line ([@&#8203;igwejk])
#### 🐛 Fixes

- [#&#8203;2662]: Replace unicode chars w/ hex codes in HTML reporter ([@&#8203;rotemdan])
#### 🔍 Coverage

- [#&#8203;2672]: Add coverage for node tests ([@&#8203;c089], [@&#8203;Munter])
- [#&#8203;2680]: Increase tests coverage for base reporter ([@&#8203;epallerols])
- [#&#8203;2690]: Increase tests coverage for doc reporter ([@&#8203;craigtaub])
- [#&#8203;2701]: Increase tests coverage for landing, min, tap and list reporters ([@&#8203;craigtaub])
- [#&#8203;2691]: Increase tests coverage for spec + dot reporters ([@&#8203;craigtaub])
- [#&#8203;2698]: Increase tests coverage for xunit reporter ([@&#8203;craigtaub])
- [#&#8203;2699]: Increase tests coverage for json-stream, markdown and progress reporters ([@&#8203;craigtaub])
- [#&#8203;2703]: Cover .some() function in utils.js with tests ([@&#8203;seppevs])
- [#&#8203;2773]: Add tests for loading reporters w/ relative/absolute paths ([@&#8203;sul4bh])
#### 🔩 Other

- Remove bin/.eslintrc; ensure execs are linted ([@&#8203;boneskull])
- [#&#8203;2542]: Expand CONTRIBUTING.md ([@&#8203;boneskull])
- [#&#8203;2660]: Double timeouts on integration tests ([@&#8203;Munter])
- [#&#8203;2653]: Update copyright year ([@&#8203;Scottkao85], [@&#8203;Munter])
- [#&#8203;2621]: Update dependencies to enable Greenkeeper ([@&#8203;boneskull], [@&#8203;greenkeeper])
- [#&#8203;2625]: Use trusty container in travis-ci; use "artifacts" addon ([@&#8203;boneskull])
- [#&#8203;2670]: doc(CONTRIBUTING): fix link to org members ([@&#8203;coderbyheart])
- Add Mocha propaganda to README.md ([@&#8203;boneskull])
- [#&#8203;2470]: Avoid test flake in "delay" test ([@&#8203;boneskull])
- [#&#8203;2675]: Limit browser concurrency on sauce ([@&#8203;boneskull])
- [#&#8203;2669]: Use temporary test-only build of mocha.js for browsers tests ([@&#8203;Munter])
- Fix "projects" link in README.md ([@&#8203;boneskull])
- [#&#8203;2678]: Chore(Saucelabs): test on IE9, IE10 and IE11 ([@&#8203;coderbyheart])
- [#&#8203;2648]: Use `semistandard` directly ([@&#8203;kt3k])
- [#&#8203;2727]: Make the build reproducible ([@&#8203;lamby])

[@&#8203;boneskull]: https://github.com/boneskull
[@&#8203;c089]: https://github.com/c089
[@&#8203;coderbyheart]: https://github.com/coderbyheart
[@&#8203;craigtaub]: https://github.com/craigtaub
[@&#8203;epallerols]: https://github.com/epallerols
[@&#8203;greenkeeper]: https://github.com/greenkeeper
[@&#8203;igwejk]: https://github.com/igwejk
[@&#8203;kt3k]: https://github.com/kt3k
[@&#8203;lamby]: https://github.com/lamby
[@&#8203;Munter]: https://github.com/Munter
[@&#8203;rotemdan]: https://github.com/rotemdan
[@&#8203;seppevs]: https://github.com/seppevs
[@&#8203;sul4bh]: https://github.com/sul4bh

[#&#8203;2470]: `https://github.com/mochajs/mocha/pull/2470`
[#&#8203;2542]: `https://github.com/mochajs/mocha/issues/2542`
[#&#8203;2621]: `https://github.com/mochajs/mocha/pull/2621`
[#&#8203;2625]: `https://github.com/mochajs/mocha/pull/2625`
[#&#8203;2648]: `https://github.com/mochajs/mocha/pull/2648`
[#&#8203;2653]: `https://github.com/mochajs/mocha/pull/2653`
[#&#8203;2659]: `https://github.com/mochajs/mocha/pull/2659`
[#&#8203;2660]: `https://github.com/mochajs/mocha/pull/2660`
[#&#8203;2662]: `https://github.com/mochajs/mocha/pull/2662`
[#&#8203;2669]: `https://github.com/mochajs/mocha/pull/2669`
[#&#8203;2670]: `https://github.com/mochajs/mocha/pull/2670`
[#&#8203;2672]: `https://github.com/mochajs/mocha/pull/2672`
[#&#8203;2675]: `https://github.com/mochajs/mocha/pull/2675`
[#&#8203;2678]: `https://github.com/mochajs/mocha/pull/2678`
[#&#8203;2680]: `https://github.com/mochajs/mocha/pull/2680`
[#&#8203;2690]: `https://github.com/mochajs/mocha/pull/2690`
[#&#8203;2691]: `https://github.com/mochajs/mocha/pull/2691`
[#&#8203;2698]: `https://github.com/mochajs/mocha/pull/2698`
[#&#8203;2699]: `https://github.com/mochajs/mocha/pull/2699`
[#&#8203;2701]: `https://github.com/mochajs/mocha/pull/2701`
[#&#8203;2703]: `https://github.com/mochajs/mocha/pull/2703`
[#&#8203;2727]: `https://github.com/mochajs/mocha/pull/2727`
[#&#8203;2769]: `https://github.com/mochajs/mocha/pull/2769`
[#&#8203;2773]: `https://github.com/mochajs/mocha/pull/2773`

---

### [`v3.2.0`](https://github.com/mochajs/mocha/blob/master/CHANGELOG.md#&#8203;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

- [#&#8203;2535]: Fix crash when `--watch` encounters broken symlinks ([@&#8203;villesau])
- [#&#8203;2593]: Fix (old) regression; incorrect symbol shown in `list` reporter ([@&#8203;Aldaviva])
- [#&#8203;2584]: Fix potential error when running XUnit reporter ([@&#8203;vobujs])
#### 🎉 Enhancement

- [#&#8203;2294]: Improve timeout error messaging ([@&#8203;jeversmann], [@&#8203;boneskull])
- [#&#8203;2520]: Add info about `--inspect` flag to CLI help ([@&#8203;ughitsaaron])
#### 🔩 Other

- [#&#8203;2570]: Use [karma-mocha](https://npmjs.com/package/karma-mocha) proper ([@&#8203;boneskull])
- Licenses updated to reflect new copyright, add link to license and browser matrix to `README.md` ([@&#8203;boneskull], [@&#8203;ScottFreeCode], [@&#8203;dasilvacontin])

[#&#8203;2294]: `https://github.com/mochajs/mocha/issues/2294`
[#&#8203;2535]: `https://github.com/mochajs/mocha/issues/2535`
[#&#8203;2520]: `https://github.com/mochajs/mocha/pull/2520`
[#&#8203;2593]: `https://github.com/mochajs/mocha/pull/2593`
[#&#8203;2584]: `https://github.com/mochajs/mocha/issues/2584`
[#&#8203;2570]: `https://github.com/mochajs/mocha/issues/2570`
[@&#8203;Aldaviva]: https://github.com/Aldaviva
[@&#8203;jeversmann]: https://github.com/jeversmann
[@&#8203;ughitsaaron]: https://github.com/ughitsaaron
[@&#8203;villesau]: https://github.com/villesau
[@&#8203;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).
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.

3 participants