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

Ember-Mocha + Ember-Cli-Mocha = invalid /tests HTML #211

Open
mike-north opened this issue Jul 31, 2018 · 3 comments
Open

Ember-Mocha + Ember-Cli-Mocha = invalid /tests HTML #211

mike-north opened this issue Jul 31, 2018 · 3 comments

Comments

@mike-north
Copy link

mike-north commented Jul 31, 2018

A project with both ember-cli-mocha and ember-mocha as top-level ember addons will result in duplicate fixture HTML being rendered. Unfortunately, this also means that more than one DOM element has the same id. This is not allowed.

screen shot 2018-07-31 at 11 20 06 am

The end result is that tests will pass, but the developer will not be able to easily inspect the rendered app in the 50% scaled frame (the second set of DOM elements) due to the real testing being done in the first set of DOM elements.

Removing ember-cli-mocha from the app's package.json resolves this problem.

Possible root cause

the contentFor hook of this addon is

ember-mocha/index.js

Lines 66 to 77 in 13be23a

contentFor(type) {
// Skip if insertContentForTestBody === false.
if (type === 'test-body' && !(this.targetOptions().insertContentForTestBody === false)) {
return stripIndent`
<div id="mocha"></div>
<div id="mocha-fixture"></div>
<div id="ember-testing-container">
<div id="ember-testing"></div>
</div>
`;
}
},

and for ember-cli-mocha
https://github.com/ember-cli/ember-cli-mocha/blob/64d8742b7fdc8535f76b545fb7fc7c6d6c97b01c/index.js#L10-L13

  contentFor(type) {
    let output = this.eachAddonInvoke('contentFor', [type]);
    return output.join('\n');
  },

It's possible that this results in ember-mocha's fixture HTML being inserted twice

@Turbo87
Copy link
Member

Turbo87 commented Mar 18, 2019

ember-cli-mocha and ember-mocha should not be installed at the same time. ember-cli-mocha already brings its own ember-mocha dependency, but ultimately we should just use ember-mocha directly from now on

@mike-north
Copy link
Author

@Turbo87 any reason why we shouldn't check for invalid dependency situations in this addon's included hook?

@Turbo87
Copy link
Member

Turbo87 commented Mar 18, 2019

no, makes sense to check it. though we need to be careful to not warn for the app -> ember-cli-mocha -> ember-mocha dependency scenario.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants