Skip to content

Add unit test towards #494 #497

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

Closed
wants to merge 1 commit into from

Conversation

nrchandan
Copy link
Contributor

This shouldn't fail ideally.

@codecov
Copy link

codecov bot commented Oct 23, 2017

Codecov Report

Merging #497 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #497   +/-   ##
=======================================
  Coverage   84.03%   84.03%           
=======================================
  Files          47       47           
  Lines        3801     3801           
  Branches      869      869           
=======================================
  Hits         3194     3194           
  Misses        607      607

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f713a2c...1ab8513. Read the comment docs.

it('cannot set an invalid date', (done) => {
let obj = new TestObject();
obj.set('when', new Date(Date.parse(null)));
Parse.Object.saveAll([obj]).fail((e) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This actually succeeds , but failed the CI as the done() is never called.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just pushed this along with the other PR. This test passes after PR #495. So, we know that the unit test covers this fail condition. However, coverage is still down. What should I do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I should also mention that I copy pasted this test case from just above this unit test. That one invokes save instead of saveAll, and passes. It is the saveAll that was misbehaving.

@flovilmart
Copy link
Contributor

You should probably put the test and the fix in the same PR. Also your test is broken, as you should have a done.fail in the ‘success’ handler if that’s your intention to test.

@nrchandan
Copy link
Contributor Author

nrchandan commented Oct 24, 2017 via email

@nrchandan
Copy link
Contributor Author

I just wanted to see if the test fails without my fix, and it does.

@nrchandan
Copy link
Contributor Author

Closing this one as it is incorporated in #495

@nrchandan nrchandan closed this Oct 24, 2017
@nrchandan nrchandan deleted the saveAll-fail branch October 24, 2017 02:02
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