Skip to content
This repository has been archived by the owner on Nov 28, 2022. It is now read-only.

add manual pretender shutdown to fix acceptance tests #342

Merged
merged 1 commit into from
Jan 5, 2017

Conversation

kevinansfield
Copy link
Member

no issue

  • ensure our mirage server is shutdown during app teardown in acceptance tests
  • a recent update to Pretender contained a breaking change that throws an error when multiple pretender instances exist
  • using mirage in acceptance tests was triggering the error

@robneville73
Copy link

@kevinansfield tried to do the same thing and it's not fixing the issue...any idea what I might be missing?

@trek
Copy link

trek commented Oct 18, 2016

Pretender 1.4.1 is released and only warns now. To be fair: starting several pretender servers has never been allowed and shutting down is specifically called out in the README as a requirement. We only started preventing it because people forgetting this requirement was causing other issues.

@acburdine
Copy link
Member

@kevinansfield Do we still want this? If so it might be a good idea to update the TODO comment :)

@acburdine
Copy link
Member

@kevinansfield I can either merge this now or close it as the same change is present in #37 as part of the mirage update. I'm fine with either :)

no issue
- a [recent update](pretenderjs/pretender#178) to Pretender contained a breaking change that throws an error when multiple pretender instances exist
- using mirage in acceptance tests was [triggering the error](miragejs/ember-cli-mirage#915)
@kevinansfield
Copy link
Member Author

@acburdine let's merge this now as it makes the test logs on Travis rather noisy. I've updated the comment to remove the TODO as the upstream "fix" was to make it a warning rather than an error meaning this is still necessary

@acburdine acburdine merged commit 000db35 into TryGhost:master Jan 5, 2017
@kevinansfield kevinansfield deleted the manual-pretender-shutdown branch February 4, 2017 09:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants