Skip to content

Conversation

@russellwheatley
Copy link
Member

@russellwheatley russellwheatley commented Aug 25, 2020

Description

Cleaned up rules so that only a single collection (firestore) & a single collection group (collectionGroup) is used for tests.

Related issues

As per this comment

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
    • Yes
  • My change supports the following platforms;
    • Android
    • iOS
  • My change includes tests;
    • e2e tests added or updated in packages/\*\*/e2e
    • jest tests added or updated in packages/\*\*/__tests__
  • I have updated TypeScript types that are affected by my change.
  • This is a breaking change;
    • Yes
    • No

Think react-native-firebase is great? Please consider supporting the project with any of the below:

@vercel
Copy link

vercel bot commented Aug 25, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/invertase/react-native-firebase/9kfgp6hkb
✅ Preview: https://react-native-firebase-git-russell-follow-up-emulator.invertase.vercel.app

@vercel vercel bot temporarily deployed to Preview August 26, 2020 07:44 Inactive
@mikehardy
Copy link
Collaborator

This looks good in general but is failing in CI:



  1) firestore.collection().add()
       adds a new document:
     NativeFirebaseError: [firestore/permission-denied] The caller does not have permission to execute the specified operation.
      at FirestoreDocumentReference.set (http://localhost:8081/index.bundle?platform=ios&dev=true&minify=false&sourceMapURL=true:188225:39)

@mikehardy mikehardy added the Workflow: Waiting for User Response Blocked waiting for user response. label Sep 4, 2020
@codecov
Copy link

codecov bot commented Oct 6, 2020

Codecov Report

Merging #4133 into master will decrease coverage by 0.06%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4133      +/-   ##
==========================================
- Coverage   67.10%   67.05%   -0.05%     
==========================================
  Files         114      114              
  Lines        3820     3820              
  Branches      278      278              
==========================================
- Hits         2563     2561       -2     
- Misses       1148     1150       +2     
  Partials      109      109              

@russellwheatley
Copy link
Member Author

russellwheatley commented Oct 8, 2020

There are two bugs associated with this PR that conspired to throw me off the scent. One is a Firestore emulator UI bug that I've filed here.

The other is a reset of the Firestore settings at this point which meant that, for iOS at least, we are still using the remote database 😅 .

@russellwheatley russellwheatley marked this pull request as ready for review October 14, 2020 11:33
@russellwheatley russellwheatley removed the Workflow: Waiting for User Response Blocked waiting for user response. label Oct 14, 2020

PODFILE CHECKSUM: 51062414d4f230ae385561d26f3c88e6c8a3ce28

COCOAPODS: 1.10.0.beta.2
Copy link
Collaborator

Choose a reason for hiding this comment

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

not terribly relevant, but cocoapods has been stuck on 1.10.0-rc.1 for 29 days now, thought for sure it would be out. Anyway, this change doesn't really matter one way or the other and is not incorrect, just a comment

Copy link
Collaborator

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

This looks great and I suppose sets us up to more easily implement the new firestore operators they just added (there's always something to do... 😅 ) - thanks, I'll shepherd it through CI and get it merged

@mikehardy mikehardy added the Workflow: Pending Merge Waiting on CI or similar label Oct 14, 2020
@mikehardy mikehardy merged commit 6b20a2d into master Oct 14, 2020
@mikehardy mikehardy deleted the @russell/follow-up-emulator branch October 14, 2020 17:44
@mikehardy mikehardy removed the Workflow: Pending Merge Waiting on CI or similar label Oct 14, 2020
androidIsForVivek pushed a commit to androidIsForVivek/react-native-firebase that referenced this pull request Aug 9, 2021
* test(firestore): clean up rules

* docs(firestore): update e2e contributing docs

* format: prettier e2e files

* test: ios updates

* testing ci ios tests

* ci ios test

* ci ios testing

* revert project.pbxproj

* test(firestore): fix broken tests

* test(firestore): rm todo

Co-authored-by: Mike Diarmid <mike.diarmid@gmail.com>
androidIsForVivek pushed a commit to androidIsForVivek/react-native-firebase that referenced this pull request Aug 9, 2021
* test(firestore): clean up rules

* docs(firestore): update e2e contributing docs

* format: prettier e2e files

* test: ios updates

* testing ci ios tests

* ci ios test

* ci ios testing

* revert project.pbxproj

* test(firestore): fix broken tests

* test(firestore): rm todo

Co-authored-by: Mike Diarmid <mike.diarmid@gmail.com>
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.

4 participants