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

[WIP] Adding more test cases #70

Open
wants to merge 4 commits into
base: master
from

Conversation

@prashant-shahi
Copy link
Member

commented Sep 9, 2019

Adding new test cases as per the internal discussion.

  • Type directive: s * * deletion
  • Type System: type function
  • Upsert: 1 mutation using Do
  • Upsert: 2 mutations using Do
  • Upsert: 1 mutation, 1 query using Do
  • Upsert: 1 query using Do
  • Upsert: 0 query, 0 mutation using Do
  • Upsert: Json Tests
  • Conditional Upsert
  • TxnConflict Exception
  • TxnReadOnly Exception
  • TxnFinished Exception
  • Bulk Set using Do
  • Bulk Delete using Do
  • ACL Test - Read
  • ACL Test - Write
  • ACL Test - Modify

This change is Reviewable

@pullrequest
Copy link

left a comment

A review job has been created and sent to the PullRequest network.


Check the status or cancel PullRequest code review here.

@pullrequest
Copy link

left a comment

1 Message
⚠️ Due to its size, this pull request will likely have a little longer turnaround time and will probably require multiple passes from our reviewers.

@coveralls

This comment has been minimized.

Copy link

commented Sep 9, 2019

Coverage Status

Coverage decreased (-3.1%) to 90.136% when pulling fa44f98 on prashant/adding-tests into 62ae0fe on master.

@prashant-shahi prashant-shahi requested review from mangalaman93 and paulftw Sep 9, 2019

@pullrequest
Copy link

left a comment

👍 nice job circling back to make sure there is good test coverage!


Reviewed with ❤️ by PullRequest

@pullrequest
Copy link

left a comment

Just one or two small items to improve future readability.


Reviewed with ❤️ by PullRequest

@@ -284,4 +298,14 @@ describe("upsert using doRequest", () => {
`);
await doUpsert();
});

it("should not perform upsert", async () => {

This comment has been minimized.

Copy link
@pullrequest

pullrequest bot Sep 10, 2019

I would give more context for this test, why exactly should it not upsert? This would give more explanation for users that edit this code in the future.

tests/integration/doRequest.spec.ts Outdated Show resolved Hide resolved
Adding more test cases
- conditional upsert test
- bulk set and delete test
- conflict/exception test
- doRequest test
@prashant-shahi
Copy link
Member Author

left a comment

Reviewable status: 0 of 6 files reviewed, 2 unresolved discussions (waiting on @mangalaman93, @paulftw, and @pullrequest[bot])


tests/integration/doRequest.spec.ts, line 20 at r1 (raw file):

Previously, pullrequest[bot] wrote…

Nitpicking: The naming on these variables could be a bit more descriptive.

Done.


tests/integration/upsert.spec.ts, line 302 at r1 (raw file):

Previously, pullrequest[bot] wrote…

I would give more context for this test, why exactly should it not upsert? This would give more explanation for users that edit this code in the future.

Done.

@prashant-shahi prashant-shahi changed the title [WIP] Adding more test cases for upsert and doRequest [WIP] Adding more test cases Sep 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.