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
allow consuming apps to use mirage factories/models/serializers #142
Conversation
README.md
Outdated
``` | ||
|
||
## Testing with Mirage | ||
|
||
This addon use [ember-cli-mirage](http://www.ember-cli-mirage.com/) in its tests. It is often beneficial for consuming apps to be able to re-use the factories and models defined in mirage, so if you would like to use these in your tests you can add the `mirage-support` object to your `ember-cli-build.js` file: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*uses
``` | ||
|
||
|
||
As long as `ember-cli-mirage` is not disabled, the files in this addon's `mirage-support` directory will be merged with the consuming app's namespace, and be made available to that mirage context. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering if it's better to link back to mirage docs for this, if it's well-explained there? Otherwise we have to keep this in sync with the source package if there are changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately this is not a first-class thing that is supported by mirage so there is nothing over there to link to. I largely took this implementation from another addon that wanted to do the same thing for its mirage stubs. This is basically manually putting the mirage files into the build tree so consuming apps can access them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok. Seems fine to leave it here for now. As extra credit, if you feel like it, it might be worth filing a feature request with mirage to support this internally so everyone can benefit from it.
addon/adapters/github-compare.js
Outdated
const { repo, base, head } = query; | ||
delete query.repo; | ||
delete query.base; | ||
delete query.head; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this approach of deleting it when the only thing that uses it has no need for it anymore.
Will review once #140 lands, since it's hard to separate the diffs as-is. |
0a3fdfc
to
2bcb8a4
Compare
2bcb8a4
to
de35f5c
Compare
Ready for review now |
Closes #141
Depends on #140
Note this came out of trying to use this addon in our production app and it wanting to have the ability to do this to write tests on our side against ember-data-github's models