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

Move sendCoverage to be added by the host in QUnit.done #332

Merged
merged 3 commits into from
Oct 28, 2021

Conversation

thoov
Copy link
Contributor

@thoov thoov commented Oct 28, 2021

After testing out the beta inside of a large internal embroider app these changes were needed:

  1. Moving sendCoverage to be invoked by the end user (e.g. in QUnit.done)
  2. Adding a default namespace so that embroider apps without symlinks work correctly (if you symlink an addon into an embroider app the workspaceDir will look like: /tmp/embroider/hash/app-namespace/app.js and /tmp/embroider/hash/linked-project-namespace/foo.js whereas without the link it looks like: /tmp/embroider/hash/app.js)
  3. Adding node_module exclusion to the default excludes as just a perf boost under embroider.

The setup for consumers changes in the following ways:

  • Remove the <script> for /coverage.js in tests/index.html
  • Update tests/test-helper.js to something like:
import Qunit from 'qunit';
import { forceModulesToBeLoaded, sendCoverage } from 'ember-cli-code-coverage/test-support';

QUnit.done(async function() {
  forceModulesToBeLoaded();
  await sendCoverage();
});

Comment on lines +100 to +103
// this adds a "default" lookup to the namespace in the event that there is no
// namespace. this comes up under embroider depending on the app structure of
// the stage 2 workspace directory. it could be either /tmp/embroider/hash/app.js
// or /tmp/embroider/hash/app-name/app.js
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems vaguely odd. I wonder if we should consider always using a consistent structure over on the Embroider side.

@thoov
Copy link
Contributor Author

thoov commented Oct 28, 2021

👍🏻

@rwjblue rwjblue changed the title Add a default namespace and restructure forceModule logic Move sendCoverage to be added by the host in QUnit.done Oct 28, 2021
@rwjblue rwjblue added breaking and removed bug labels Oct 28, 2021
@rwjblue rwjblue merged commit f24f8a2 into ember-cli-code-coverage:master Oct 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants