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

Npm (5) install will override the symlinked addon under test #176

Open
simonihmig opened this issue Jan 2, 2018 · 2 comments
Open

Npm (5) install will override the symlinked addon under test #176

simonihmig opened this issue Jan 2, 2018 · 2 comments

Comments

@simonihmig
Copy link
Collaborator

Not a bug here, but in npm itself: npm/npm#17287

Lost quite some time because of this, so mentioning this here in case others get bitten by this. ember-cli-addon-tests will symlink the addon under test into node_modules of the temporary app, but if you call npm install afterwards (e.g. by calling app.run('npm', 'install')), npm 5 will remove the symlink and install the addon from the npm registry (which is probably not the same code that you are working on). Does not happen with npm 3.

@kellyselden
Copy link
Collaborator

All I have to say is WTF.

adamjmcgrath added a commit to ember-cli-code-coverage/ember-cli-code-coverage that referenced this issue Feb 6, 2018
RobbieTheWagner added a commit to ember-cli-code-coverage/ember-cli-code-coverage that referenced this issue Feb 6, 2018
* Start in-repo addon tests

* Update per Adam's suggestion

* Fix import paths

* fix babel-plugin-istanbul caching issue (#159)

* fix babel-plugin-istanbul caching issue

* - Refactor instrumentation logic given `babel-plugin-istanbul` constraints
- Fix Unit tests

* Update app-coverage-test.js

* Update in-repo-addon-coverage-test.js

* Try setting path to process.cwd

* Fix tests to workaround tomdale/ember-cli-addon-tests#176
@mansona
Copy link

mansona commented May 14, 2018

Ok so I've done some investigation into this and I don't really know how to proceed. The main issue is that the link needs to be the last thing that is done after all npm install steps.

@simonihmig you are right saying this is a known issue in npm but the main issue is this one npm/npm#10343 which is tagged with big-bug so it's unlikely that we'll get an "out of the box" fix any time soon, although I happen to know that npm@6 had an internal refactoring that was specifically designed to address issues like this.

I will create a PR to show my work so far, although it is specifically related to kaliber5/ember-fastboot-addon-tests#21

RobbieTheWagner pushed a commit to ember-cli-code-coverage/ember-cli-code-coverage that referenced this issue Oct 15, 2018
* Update CHANGELOG for 0.3.12.

* 0.3.12

* Improve ES7 error message

This messaging was confusing to me so I took a stab at clarifying the intent.

* add mocha support and fix pretender bug

* [Fixes #111] Update dependencies, get babel instrumenter working (#115)

babel 6 fix

* test

* test

* add mocha support and fix pretender bug

[Fixes #111] Update dependencies, get babel instrumenter working (#115)

babel 6 fix

* 0.4.0

* update changelog v0.4.0

* Fixes the hanging issue of #88 (#90)

* Refactor onload to onreadystatechange

onreadystatechange is more reliable, chromium doesnt trigger onload

* Fix PhantomJS honoring responseType issue

* 0.4.1

* update changelog v0.4.1

* Suggest using `posttest` hook for `ember coverage-merge`

It might be helpful to suggest how/where to run `coverage-merge` for people looking to set this up in a CI environment.

The only other thing I might add is that you need to run `npm test` and have the appropriate configuration there for `posttest` to trigger, but I'm not sure if that's overkill since that should be common knowledge.

* Resolve addon file paths correctly in CLI >= 2.12

* Spelling Fix

* support nested coverageFolder

* fix test

* Update minimum version of ember-cli-babel.

The previously locked version was not compatible with `node@8` (due to
`engines` shenanigans).

* Add babel-plugin-istanbul dep.

* Use babel-plugin-istanbul instead of custom instrumenter.

This has some negative effects still:

* Does not re-write the paths to match "real" on disk paths
* Does not instrument dummy app files (I think)

Even with these negative side-effects, it has massive upside:

* Massively less overall code to maintain
* Does not require us to parse babel config (and therefore avoids issues
  around parallelism in broccoli-babel-transpiler)
* Significantly faster when used (e.g. no longer has to double parse and
  process files)

* Refactor middleware to use new istanbul API.

* Refactor coverage-merge command to work with new istanbul-api.

* Add node badge

Figured it would be nice to have a badge for the node version, so people can quickly see what the latest release is.

* 0.4.2

* Fix paths for istanbul report and remove parallel logic for impicit parallel support

* Delete uneeded files

* Remove component fixtures and add tests for 'excludes' config

* Add support for in-repo-addons #120

* Revert parallel changes (TODO: move to another PR)

* Only include test fixtures when testing the addon.

* Add index.js unit tests

* Update some docs

* Add comment about .istanbul.yml to README

* Fix typo

* Bump ember, fix lint

* Adjust some deps

* Bump ember-cli-release

* Add ember-cli-changelog

* Released v1.0.0-beta.0

* Add back "Avoid throwing errors while requiring files for coverage" #64

See #63
Fixes #150

* Upgrade out-of-date deps

* Update sinon

* Setup travis ci to release on pushed tag, add lerna-changelog

- Documented in RELEASE.md

* Add v1.0.0-beta.1 to CHANGELOG
[ci skip]

* 1.0.0-beta.1

* Don't restrict travis to particular branches

* Try using travis stages

* Reformat .travis.yml and do not require sudo

* Pin auto-dist-tag and add --no-sandbox to chrome args if on travis

* Split out script for use with matrix build

* Adjust deploy config

* Make fixtures external (#156)

* Start moving test files to separate addon

* First pass at ember-cli-addon-tests

* Fix test

* Remove addon test, fix lint

* Remove treeFor

* Try sudo required

* Remove no sandbox

* Drop node 4 from travis, use npm instead of yarn

* Add 8

* Add filter

* Add testem.js to fixtures

* Add eslint

* Remove eslint plugins

* Start in-repo addon tests (#158)

* Start in-repo addon tests

* Update per Adam's suggestion

* Fix import paths

* fix babel-plugin-istanbul caching issue (#159)

* fix babel-plugin-istanbul caching issue

* - Refactor instrumentation logic given `babel-plugin-istanbul` constraints
- Fix Unit tests

* Update app-coverage-test.js

* Update in-repo-addon-coverage-test.js

* Try setting path to process.cwd

* Fix tests to workaround tomdale/ember-cli-addon-tests#176

* Update CHANGELOG for v1.0.0-beta.2.

* 1.0.0-beta.2

* Support for addon-test-support (#160)

* Start on support for addon-test-support

* Remove only so all tests run

* Try adding test-support prefix

* Ensure addon-test-support coverage

* Add tests for in-repo engines (#162)

* First attempt at in-repo-engine

* Fix engine coverage test

* Fix lint

* Update CHANGELOG for v1.0.0-beta.3.

* 1.0.0-beta.3

* Remove merge-coverage and explicit parallel option (#163)

* Remove merge-coverage and explicit parallel option

* fix lint

* Update README.md

Describe how parallel works

* Revert "Remove merge-coverage and explicit parallel option (#163)"

This reverts commit 0592f5f.

* Keep implicit and explicit parallel logic

* Update babel-plugin-istanbul (#169)

* Use the parent registry for determining JS extensions (#164)

* Removing unused dependency exists-sync which fixes the deprecation warning from ember-cli (#179)

* Update CHANGELOG for v1.0.0-beta.4.

* 1.0.0-beta.4

* Fix fileLookup is null in testemMiddleware (#182)

* upgrade istanbul-api to 2.0.1 (#186)

[email protected] was released on June 6, 2018 and tagged as "next" on npm. The breaking change is pretty straightforward: https://github.com/istanbuljs/istanbuljs/blob/master/packages/istanbul-api/CHANGELOG.md#breaking-changes

* Filter out in-repo addons that could not be found (#188)

* Ember 3.4 (#190)

* Bump deps (#191)

* Bump deps

* Reset engine test versions

* Update CHANGELOG for v1.0.0-beta.5

* 1.0.0-beta.5

* Do not publish coverage, tests, or .idea to npm (#192)

* Update babel-plugin-istanbul (#194)

* Update CHANGELOG for v1.0.0-beta.6

* 1.0.0-beta.6

* TypeScript integration (howto) (#196)

* TypeScript integration (howto)

* removed some abstraction

* 1.0.0-beta.6

* fix: handle babel 7 absolute paths (#199)
NullVoxPopuli added a commit to NullVoxPopuli/ember-cli-code-coverage that referenced this issue Oct 27, 2019
* Update CHANGELOG for 0.3.12.

* 0.3.12

* Improve ES7 error message

This messaging was confusing to me so I took a stab at clarifying the intent.

* add mocha support and fix pretender bug

* [Fixes ember-cli-code-coverage#111] Update dependencies, get babel instrumenter working (ember-cli-code-coverage#115)

babel 6 fix

* test

* test

* add mocha support and fix pretender bug

[Fixes ember-cli-code-coverage#111] Update dependencies, get babel instrumenter working (ember-cli-code-coverage#115)

babel 6 fix

* 0.4.0

* update changelog v0.4.0

* Fixes the hanging issue of ember-cli-code-coverage#88 (ember-cli-code-coverage#90)

* Refactor onload to onreadystatechange

onreadystatechange is more reliable, chromium doesnt trigger onload

* Fix PhantomJS honoring responseType issue

* 0.4.1

* update changelog v0.4.1

* Suggest using `posttest` hook for `ember coverage-merge`

It might be helpful to suggest how/where to run `coverage-merge` for people looking to set this up in a CI environment.

The only other thing I might add is that you need to run `npm test` and have the appropriate configuration there for `posttest` to trigger, but I'm not sure if that's overkill since that should be common knowledge.

* Resolve addon file paths correctly in CLI >= 2.12

* Spelling Fix

* support nested coverageFolder

* fix test

* Update minimum version of ember-cli-babel.

The previously locked version was not compatible with `node@8` (due to
`engines` shenanigans).

* Add babel-plugin-istanbul dep.

* Use babel-plugin-istanbul instead of custom instrumenter.

This has some negative effects still:

* Does not re-write the paths to match "real" on disk paths
* Does not instrument dummy app files (I think)

Even with these negative side-effects, it has massive upside:

* Massively less overall code to maintain
* Does not require us to parse babel config (and therefore avoids issues
  around parallelism in broccoli-babel-transpiler)
* Significantly faster when used (e.g. no longer has to double parse and
  process files)

* Refactor middleware to use new istanbul API.

* Refactor coverage-merge command to work with new istanbul-api.

* Add node badge

Figured it would be nice to have a badge for the node version, so people can quickly see what the latest release is.

* 0.4.2

* Fix paths for istanbul report and remove parallel logic for impicit parallel support

* Delete uneeded files

* Remove component fixtures and add tests for 'excludes' config

* Add support for in-repo-addons ember-cli-code-coverage#120

* Revert parallel changes (TODO: move to another PR)

* Only include test fixtures when testing the addon.

* Add index.js unit tests

* Update some docs

* Add comment about .istanbul.yml to README

* Fix typo

* Bump ember, fix lint

* Adjust some deps

* Bump ember-cli-release

* Add ember-cli-changelog

* Released v1.0.0-beta.0

* Add back "Avoid throwing errors while requiring files for coverage" ember-cli-code-coverage#64

See ember-cli-code-coverage#63
Fixes ember-cli-code-coverage#150

* Upgrade out-of-date deps

* Update sinon

* Setup travis ci to release on pushed tag, add lerna-changelog

- Documented in RELEASE.md

* Add v1.0.0-beta.1 to CHANGELOG
[ci skip]

* 1.0.0-beta.1

* Don't restrict travis to particular branches

* Try using travis stages

* Reformat .travis.yml and do not require sudo

* Pin auto-dist-tag and add --no-sandbox to chrome args if on travis

* Split out script for use with matrix build

* Adjust deploy config

* Make fixtures external (ember-cli-code-coverage#156)

* Start moving test files to separate addon

* First pass at ember-cli-addon-tests

* Fix test

* Remove addon test, fix lint

* Remove treeFor

* Try sudo required

* Remove no sandbox

* Drop node 4 from travis, use npm instead of yarn

* Add 8

* Add filter

* Add testem.js to fixtures

* Add eslint

* Remove eslint plugins

* Start in-repo addon tests (ember-cli-code-coverage#158)

* Start in-repo addon tests

* Update per Adam's suggestion

* Fix import paths

* fix babel-plugin-istanbul caching issue (ember-cli-code-coverage#159)

* fix babel-plugin-istanbul caching issue

* - Refactor instrumentation logic given `babel-plugin-istanbul` constraints
- Fix Unit tests

* Update app-coverage-test.js

* Update in-repo-addon-coverage-test.js

* Try setting path to process.cwd

* Fix tests to workaround tomdale/ember-cli-addon-tests#176

* Update CHANGELOG for v1.0.0-beta.2.

* 1.0.0-beta.2

* Support for addon-test-support (ember-cli-code-coverage#160)

* Start on support for addon-test-support

* Remove only so all tests run

* Try adding test-support prefix

* Ensure addon-test-support coverage

* Add tests for in-repo engines (ember-cli-code-coverage#162)

* First attempt at in-repo-engine

* Fix engine coverage test

* Fix lint

* Update CHANGELOG for v1.0.0-beta.3.

* 1.0.0-beta.3

* Remove merge-coverage and explicit parallel option (ember-cli-code-coverage#163)

* Remove merge-coverage and explicit parallel option

* fix lint

* Update README.md

Describe how parallel works

* Revert "Remove merge-coverage and explicit parallel option (ember-cli-code-coverage#163)"

This reverts commit 0592f5f.

* Keep implicit and explicit parallel logic

* Update babel-plugin-istanbul (ember-cli-code-coverage#169)

* Use the parent registry for determining JS extensions (ember-cli-code-coverage#164)

* Removing unused dependency exists-sync which fixes the deprecation warning from ember-cli (ember-cli-code-coverage#179)

* Update CHANGELOG for v1.0.0-beta.4.

* 1.0.0-beta.4

* Fix fileLookup is null in testemMiddleware (ember-cli-code-coverage#182)

* upgrade istanbul-api to 2.0.1 (ember-cli-code-coverage#186)

[email protected] was released on June 6, 2018 and tagged as "next" on npm. The breaking change is pretty straightforward: https://github.com/istanbuljs/istanbuljs/blob/master/packages/istanbul-api/CHANGELOG.md#breaking-changes

* Filter out in-repo addons that could not be found (ember-cli-code-coverage#188)

* Ember 3.4 (ember-cli-code-coverage#190)

* Bump deps (ember-cli-code-coverage#191)

* Bump deps

* Reset engine test versions

* Update CHANGELOG for v1.0.0-beta.5

* 1.0.0-beta.5

* Do not publish coverage, tests, or .idea to npm (ember-cli-code-coverage#192)

* Update babel-plugin-istanbul (ember-cli-code-coverage#194)

* Update CHANGELOG for v1.0.0-beta.6

* 1.0.0-beta.6

* TypeScript integration (howto) (ember-cli-code-coverage#196)

* TypeScript integration (howto)

* removed some abstraction

* 1.0.0-beta.6

* fix: handle babel 7 absolute paths (ember-cli-code-coverage#199)
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

No branches or pull requests

3 participants