Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

Updating User model tests for synchronous test and fixing done() calls #768

Merged
merged 1 commit into from
Aug 6, 2015

Conversation

lirantal
Copy link
Member

@lirantal lirantal commented Aug 6, 2015

refactoring the async nature in the user model tests to account for mocha 2 second timeouts causing travis-ci build fails

@lirantal lirantal added this to the 0.4.x milestone Aug 6, 2015
@@ -153,6 +150,8 @@ describe('User Model Unit Tests:', function () {
});

after(function (done) {
User.remove().exec(done);
User.remove().exec(function() {
Copy link
Member

Choose a reason for hiding this comment

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

I believe you can just do User.remove().exec(done);.

Copy link
Member Author

Choose a reason for hiding this comment

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

you're right but I noticed some odd inconsistency with the after() call failing sometimes and wasn't sure if it is attributed to that or not.
WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

When doing User.remove().exec(done);, the after() would fail? That seems weird..I've never run into that issue and it's not like something crazy is happening. Does it work consistently in other callbacks? What about in the afterEach() below?

Copy link
Member Author

Choose a reason for hiding this comment

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

not consistently unfortunately :(
I don't mind returning to the original .exec(done) and let's keep an eye really close on tests, there is a lot more work required there to get the testing automation safety-net to be really bullet proof.

Ok with you?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah sounds good. Are you going to change it in the routes afterEach as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, if we change the convention to just .exec(done) then I'll change it back as it was to be consistent.

…ocha 2 second timeouts causing travis-ci build fails
@lirantal lirantal force-pushed the feature/users_module_tests_2 branch from 2363371 to c967a98 Compare August 6, 2015 13:52
@lirantal
Copy link
Member Author

lirantal commented Aug 6, 2015

@ilanbiala I updated and squashed, this should solve still the async/sync issue with that one test.

@codydaig
Copy link
Member

codydaig commented Aug 6, 2015

LGTM

@lirantal
Copy link
Member Author

lirantal commented Aug 6, 2015

Thanks, I'll merge.

lirantal added a commit that referenced this pull request Aug 6, 2015
Updating User model tests for synchronous test and fixing done() calls
@lirantal lirantal merged commit 0f3259f into meanjs:master Aug 6, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants