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 temporary test-only build of mocha.js for browsers tests #2669

Merged
merged 1 commit into from
Jan 19, 2017

Conversation

Munter
Copy link
Contributor

@Munter Munter commented Jan 16, 2017

This PR does the following things:

  • Creates a new BUILDTMP/mocha.js temporary file during test run, used by karma test only
  • Deletes BUILDTMP after successful run
  • Changed all make test run targets to depend on the new temporary file instead of an updated dist file
  • Added BUILDTMP to all relevant ignore lists
  • Added npm preversion hook to run tests and create the mocha.js distribution build artefact, plus stage it, so the build artifacts become a part of the release created by npm version

The overall goal of this PR is to only ever have the mocha.js browser build artefact generated when creating a new release. We want to avoid that contributors accidentally generate the build artefact during a normal test run and then accidentally check in the updated artefact.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 9a6a93b on avoid-dist-file-modification-in-test into ** on master**.

@c089
Copy link
Contributor

c089 commented Jan 18, 2017

👍 That changing mocha.js file confused me initially, so this should reduce barrier for new contributors.

What's the reason to use a fixed tmp dir in the repo instead of having one created in the OS tmp directory (e.g. using mktemp)? That way OS could take care of tmp cleanup and we wouldn't pollute the working copy.

@Munter
Copy link
Contributor Author

Munter commented Jan 18, 2017

mktemp is a unix-only tool. We eventually need windows support for everything. Debugging the build pipeline also becomes easier with human readable and accessible locations imo

@Munter
Copy link
Contributor Author

Munter commented Jan 18, 2017

Come on saucelabs! I've been restarting this job at least 5 times now :(

@c089
Copy link
Contributor

c089 commented Jan 18, 2017

oh, windows. I keep forgetting about that ;)

@ScottFreeCode
Copy link
Contributor

In my mind, there are two reasons to use an OS-managed temp file location over a local one of some sort:

  • The OS/sysadmin can clean up old files there without having to know about every project that's using temporary files.
  • You avoid cluttering your local [project, documents, whatever it is] with temporary files.

The second reason doesn't apply if you put all the temp files in a local place specifically dedicated to temp files, or if you only have a small predefined set of files with reasonable names. The first reason doesn't apply if you've got a predefined set of files (so it's not going to keep eating up more space indefinitely if not cleaned up) and the local folder is inside the relevant project/system/whatever (so removing or uninstalling the thing will take its temp files with it).

In other words, to my knowledge there's no reason for Mocha to use the OS temp file system for this, regardless of feasibility.

Now, I can name some popular subsystems that have caches in dedicated, separate-from-the-subsystem yet subsystem-specific locations that I wish would use the OS temp directory instead, so that I don't have to remember to manage that subsystem's cache space. (Granted, there's a distinction between "temporary file" and "cache", but in either case they're supposed to be able to be cleaned out from time to time without affecting the correctness of the program -- and need to be if you don't have indefinite disc space to blow on it.) But that's quite a different situation! ;^)

…accidental check-ins of dist file in contributions
@Munter Munter force-pushed the avoid-dist-file-modification-in-test branch from 9a6a93b to 8a93a54 Compare January 18, 2017 22:10
Copy link
Contributor

@ScottFreeCode ScottFreeCode left a comment

Choose a reason for hiding this comment

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

This looks good to me assuming it works.

@@ -18,8 +18,9 @@ TESTS = $(shell find test -name "*.js" -type f | sort)

all: mocha.js

mocha.js: $(SRC) browser-entry.js
mocha.js BUILDTMP/mocha.js: $(SRC) browser-entry.js
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's worth noting that this makes the directory quite obvious (not only is it not a dot-directory like .buildtmp, but it's in CAPs); I assume this is intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On a successful test run the directory is cleaned out. This happens in the npm test script. Changing the directory name is a minor thing. I'm open to suggestions of something better

@coveralls
Copy link

Coverage Status

Coverage remained the same at 76.779% when pulling 8a93a54 on avoid-dist-file-modification-in-test into 52a1957 on master.

@Munter Munter merged commit 4eada55 into master Jan 19, 2017
@Munter Munter deleted the avoid-dist-file-modification-in-test branch January 19, 2017 10:18
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).
sgilroy pushed a commit to TwineHealth/mocha that referenced this pull request Feb 27, 2019
…tion-in-test

Use temporary test-only build of mocha.js for browsers tests
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.

4 participants