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

Refactor tests. #40

Merged
merged 5 commits into from Oct 16, 2017

Conversation

Projects
None yet
3 participants
@mrded
Contributor

mrded commented Oct 12, 2017

Hello,

Can you please have a look on my changes to your tests.

  • Split up unit/ORM.test by create, associations, scope.
  • Rename UserGroup -> Group model.
  • Rename association fields: user.group -> user.groupId, image.owner -> image.userId.

I'm currently working on nested create feature, and I'll need these changes for it.

Thank you!

@mrded

This comment has been minimized.

Show comment
Hide comment
@mrded

mrded Oct 12, 2017

Contributor

It works on my machine ¯\_(ツ)_/¯

Contributor

mrded commented Oct 12, 2017

It works on my machine ¯\_(ツ)_/¯

@mrded

This comment has been minimized.

Show comment
Hide comment
@mrded

mrded Oct 13, 2017

Contributor

Actually, I've decided to implement nested creates in a separate module: https://github.com/mrded/sails-sequelize-nested

So, it's up to you, if you want to merge my changes, I don't need them any longer.

It will also be very useful if you can give me a feedback about the problem.

Thank you.

Contributor

mrded commented Oct 13, 2017

Actually, I've decided to implement nested creates in a separate module: https://github.com/mrded/sails-sequelize-nested

So, it's up to you, if you want to merge my changes, I don't need them any longer.

It will also be very useful if you can give me a feedback about the problem.

Thank you.

@KSDaemon

This comment has been minimized.

Show comment
Hide comment
@KSDaemon

KSDaemon Oct 13, 2017

Owner

Hi! Please merge latest master into your branch. I think, tests now should pass.
It's an npm issue.

Owner

KSDaemon commented Oct 13, 2017

Hi! Please merge latest master into your branch. I think, tests now should pass.
It's an npm issue.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Oct 13, 2017

Coverage Status

Coverage remained the same at 84.967% when pulling 5dba96d on mrded:tests into f938ae4 on KSDaemon:master.

coveralls commented Oct 13, 2017

Coverage Status

Coverage remained the same at 84.967% when pulling 5dba96d on mrded:tests into f938ae4 on KSDaemon:master.

@KSDaemon KSDaemon merged commit 2562d53 into KSDaemon:master Oct 16, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 84.967%
Details

@mrded mrded deleted the mrded:tests branch Oct 27, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment