emberjs / ember-mocha

Mocha helpers for testing Ember.js applications
Apache License 2.0
130 stars 69 forks source link

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

Open mike-north opened 6 years ago

mike-north commented 6 years ago

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 https://github.com/emberjs/ember-mocha/blob/13be23a36a6af0f6f1cffe934b905a2bbe1fd39e/index.js#L66-L77

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 commented 5 years ago

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 commented 5 years ago

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

Turbo87 commented 5 years ago

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.