kimroen / ember-cli-document-title

Adding document title behaviour to your ember app
MIT License
215 stars 61 forks source link

Installing causes certain Sinon tests to fail #77

Closed BitBrit closed 5 years ago

BitBrit commented 6 years ago

Repro:

  1. Create a test with a sinon.rejects() stub in it. Check that it passes.
  2. Install ember-cli-document-title
  3. Rerun test, it will now fail with an unexpected error.

I have broken down what in the addon causes the error and traced it to this line: var Promise = Ember.RSVP.Promise; In vendor/document-title/document-title.js.

I believe the problem is similar to what is described here:

https://github.com/emberjs/ember-qunit/issues/300

Has anyone experienced this before? Or implemented any fixes? I'd imagine that the simplest fix is just to either remove the line completely and use native promises in the addon or change the name of the variable into which Ember.RSVP is imported.

YoranBrondsema commented 6 years ago

I saw similar issues in my test suite, although not with sinon.rejects(). I fixed them by wrapping the statements that cause the assertion to fail with run() and adding await settled(); right after, before the assertion.

MbJav commented 5 years ago

Is anyone else still experiencing this issue/have other solutions?

YoranBrondsema commented 5 years ago

Good you ask. We transitioned to the fork https://github.com/mike-north/ember-cli-document-title-northm as this repo is no longer maintained. That fork merged https://github.com/mike-north/ember-cli-document-title-northm/pull/35, which solves the issue.

@BitBrit I think you can close the issue as using https://github.com/mike-north/ember-cli-document-title-northm solves it.

BitBrit commented 5 years ago

Yep, sounds like we don't need this if we have the fork. Incidentally, I have started using ember-page-title.