IjzerenHein / firestorter

Use Google Firestore in React with zero effort, using MobX 🀘
http://firestorter.com
MIT License
378 stars 50 forks source link

Some test fixes #38

Closed imikemiller closed 6 years ago

imikemiller commented 6 years ago

Added firestore seeds to get Collection.query.test.js and Document.reactivePath.test.js to pass. Not sure if this is the right approach. I considered putting the seeds up top in beforeAll and dumping them in afterAll but I didn't want to taint the rest of the tests so they are currently being seeded and dropped where I could see they were needed.

codecov-io commented 6 years ago

Codecov Report

Merging #38 into master will increase coverage by 4.47%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #38      +/-   ##
==========================================
+ Coverage   78.45%   82.92%   +4.47%     
==========================================
  Files           6        6              
  Lines         492      492              
  Branches      121      121              
==========================================
+ Hits          386      408      +22     
+ Misses         91       74      -17     
+ Partials       15       10       -5
Impacted Files Coverage Ξ”
src/Collection.js 81.42% <0%> (+1.58%) :arrow_up:
src/Document.js 83.33% <0%> (+1.72%) :arrow_up:
src/Utils.js 78.94% <0%> (+78.94%) :arrow_up:

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 9297b7b...2413160. Read the comment docs.

imikemiller commented 6 years ago

@IjzerenHein I have added the seed to the top of the tests but they are still failing. The re-fetch test fails and seems to be similar issue to the empty docs nonsense you were helping me with. Having said that if I run npm test -- -t 're-fetch' it passes which is confusing

IjzerenHein commented 6 years ago

Hey man, thanks for this Pull Request. I've decided to only use the relevant bits and re-implement the seed process for the sample-data differently. The PR its-self felt a bit messy, with unnecessary imports being added and duplicate seeds. I've instead created a separate step for seeding the database, which is executed once before running the jest tests. When running yarn test the sample-data in test.sampleData.json is written to the firestore database, after which the tests are run. This makes running the tests a lot faster, and you can still run the tests without the seed-process (useful when debugging), by using yarn jest. Anyway, appreciate the help a lot. Cheers, Hein

IjzerenHein commented 6 years ago

@IjzerenHein I have added the seed to the top of the tests but they are still failing. The re-fetch test fails and seems to be similar issue to the empty docs nonsense you were helping me with. Having said that if I run npm test -- -t 're-fetch' it passes which is confusing

So, I've tried to reproduce the issue you mentioned but am unable to. Could you maybe retest this with the latest master branch? For me, running wither npm test or npm test -- -t 're-fetch' always succeeds.

IjzerenHein commented 6 years ago

Also, thanks for the Donation mike, appreciate it a lot. Will add you to the list of sponsors πŸ‘

imikemiller commented 6 years ago

Yeah dude it was a bit dog eared. Should have gone over it before pushing. Whatever minor contribution I can add I will happily, the library is great

On Wed, 3 Oct 2018 at 11:31, Hein Rutjes notifications@github.com wrote:

Hey man, thanks for this Pull Request. I've decided to only use the relevant bits and re-implement the seed process for the sample-data differently. The PR its-self felt a bit messy, with unnecessary imports being added and duplicate seeds. I've instead created a separate step for seeding the database, which is executed once before running the jest tests. When running yarn test the sample-data in test.sampleData.json is written to the firestore database, after which the tests are run. This makes running the tests a lot faster, and you can still run the tests without the seed-process (useful when debugging), by using yarn jest. Anyway, appreciate the help a lot. Cheers, Hein

β€” You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/IjzerenHein/firestorter/pull/38#issuecomment-426571132, or mute the thread https://github.com/notifications/unsubscribe-auth/AXtHNpNbcA1Dc7cjMQWdCV8OXC-ipU--ks5uhIPqgaJpZM4W9_Cs .

IjzerenHein commented 6 years ago

Thanks dude!