-
-
Notifications
You must be signed in to change notification settings - Fork 136
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
Use assert rather than warn for missing traits #266
Use assert rather than warn for missing traits #266
Conversation
67a257a
to
3e03303
Compare
@@ -263,7 +263,7 @@ test("with errors in response", function(assert) { | |||
const done = assert.async(); | |||
|
|||
const response = { errors: { description: ['bad'] } }; | |||
const mock = mockFindRecord('profile', 1).fails({ response }); | |||
const mock = mockFindRecord('profile').fails({ response }); |
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.
Funnily the change exposed this
WARNING: [ember-data-factory-guy] You're trying to use a trait [1] for model profile but that trait can not be found.
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.
oooo .. that is so interesting .. that is vestige of past , the way it was used .. good catch ..
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.
This will break my tests that have these warnings ( that I have been ignoring ) .. so I know one person that will be affected. but yeah .. maybe I will bump up the ember-data version to 2.10 and make a new version 2.10
@@ -263,7 +263,7 @@ test("with errors in response", function(assert) { | |||
const done = assert.async(); | |||
|
|||
const response = { errors: { description: ['bad'] } }; | |||
const mock = mockFindRecord('profile', 1).fails({ response }); | |||
const mock = mockFindRecord('profile').fails({ response }); |
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.
oooo .. that is so interesting .. that is vestige of past , the way it was used .. good catch ..
was there another thing you were going to use assert for .. you want to do that one and then I put that out in next 2.10 release ? |
Yeah was hoping to complete #264 alongside but it got a little involved... It'll probably be next week before I get back to it. |
We kinda discussed using assertions rather than warnings for missing traits in another PR. I decided to spike this out.
If this is merged it might be worth flagging this - maybe this change is bigger than a patch release. It's possible it will cause test failures for people where before they were seeing (but perhaps not noticing or acting on) warnings that were there all along.