Skip to content
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

[CHORE tests] modernize unit/model/lifecycle-callbacks-test #5717

Merged
merged 2 commits into from
Oct 31, 2018

Conversation

runspired
Copy link
Contributor

ported from #5708

@runspired runspired added the code-review Tracks PRs that require a code-review label Oct 26, 2018
@runspired runspired requested a review from igorT October 26, 2018 06:51
@runspired runspired force-pushed the modernize-lifecycle-tests branch from 65634f7 to 4801ee4 Compare October 26, 2018 16:48
assert.strictEqual(
lifecycleEventMethodCalls,
1,
'We do not trigger didDelete when we first call record.deleteRecord'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do trigger didDelete

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

return person.save().catch(reason => {
assert.ok(reason.isAdapterError, 'reason should have been an adapter error');
await person.save().catch(reason => {
assert.strictEqual(lifecycleEventMethodCalls, 1, 'We trigger becameInvalid when we reject');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, what are the reasons for preferring strictEqual over equal in some cases?

Copy link
Contributor Author

@runspired runspired Oct 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use strictEqual when we do expect strict referential equality. The advantage is two-fold: (1) it avoids a set of bugs that happen when qunit attempts to do deep recursive "looks like the same thing" comparisons when strict equality is not matched (on records this is a mega perf hit in addition to being risky), and (2) it is an improvement over the alternative pattern of assert.ok(foo === bar, '') wherein if the assertion fails there is no additional information printed about what the incorrect value is without stepping into the test.

@igorT igorT removed the code-review Tracks PRs that require a code-review label Oct 31, 2018
@runspired runspired merged commit cec0cfa into master Oct 31, 2018
@runspired runspired deleted the modernize-lifecycle-tests branch October 31, 2018 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants