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

Add Transaction mocks #38

Merged
merged 8 commits into from
Sep 18, 2020
Merged

Add Transaction mocks #38

merged 8 commits into from
Sep 18, 2020

Conversation

AverageHelper
Copy link
Contributor

Description

We now mock over Firestore.Transaction. This branch adds mocks (all exported through firestore.js as usual) for the usual firestore.transaction API calls.

Related issues

Depends on #37

How to test

jest

This was referenced Aug 4, 2020
@AverageHelper AverageHelper marked this pull request as ready for review September 14, 2020 15:17
@AverageHelper AverageHelper mentioned this pull request Sep 14, 2020
@AverageHelper
Copy link
Contributor Author

Fixed merge conflicts in firestore.js

const mockUpdateTransaction = jest.fn();
const mockDeleteTransaction = jest.fn();

class Transaction {
Copy link
Owner

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@AverageHelper AverageHelper Sep 15, 2020

Choose a reason for hiding this comment

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

It definitely should! I must've overlooked that.

The logic for that might be complicated. Could we perhaps add it in another PR after this series is through?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's probably just a loop over each provided ref. It shouldn't be too bad to throw that in this branch. Gimme a sec...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added in the latest commit. What do you think?

Copy link
Owner

@sbatson5 sbatson5 left a comment

Choose a reason for hiding this comment

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

Thank you for adding all of this 🙌

mocks/transaction.js Show resolved Hide resolved
@sbatson5 sbatson5 merged commit 4a29c51 into sbatson5:master Sep 18, 2020
@AverageHelper AverageHelper deleted the pr-3 branch September 18, 2020 22:14
@AverageHelper
Copy link
Contributor Author

Thanks! #39 is ready for review

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