Skip to content

Conversation

@eweitz
Copy link
Member

@eweitz eweitz commented Oct 30, 2019

This refactors tests to not use the Firestore emulator, following the team's determination that Firestore does not suit our needs. It basically undoes #26.

Like before emulator integration work, tests now intercept calls to methods that load to the database. This decreases coverage and confidence slightly, but makes the tests agnostic to database technology while still being quite robust to changes in Ingest Pipeline's internal implementation.

This satisfies SCP-1968.

@eweitz eweitz force-pushed the ew-remove-firestore-emulator branch from 2d7ddd6 to 9143fcd Compare October 30, 2019 17:46
@codecov
Copy link

codecov bot commented Oct 30, 2019

Codecov Report

Merging #44 into master will increase coverage by 1.54%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #44      +/-   ##
=========================================
+ Coverage   65.15%   66.7%   +1.54%     
=========================================
  Files          11      11              
  Lines        1191     919     -272     
=========================================
- Hits          776     613     -163     
+ Misses        415     306     -109
Impacted Files Coverage Δ
ingest/ingest_pipeline.py 43.41% <100%> (-3.52%) ⬇️
ingest/subsample.py 22.72% <0%> (-69.34%) ⬇️
ingest/gene_data_model.py 59.57% <0%> (-2.13%) ⬇️
ingest/cell_metadata.py 87.69% <0%> (-1.72%) ⬇️
ingest/validation/validate_metadata.py 83.05% <0%> (-1.42%) ⬇️
ingest/validation/validate_ontology_terms.py
ingest/__init__.py 0% <0%> (ø)
ingest/clusters.py 26.66% <0%> (+0.51%) ⬆️
ingest/ingest_files.py 87.37% <0%> (+0.65%) ⬆️

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 36544aa...4371fce. Read the comment docs.

Copy link
Contributor

@jlchang jlchang left a comment

Choose a reason for hiding this comment

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

I confess, until I mock something myself I suspect I won't be able to skim through a PR on mocked responses quickly...
Knowing this is blocking Eno's progress, I'm not going to let my lack of expertise get in the way.

Other than some double quotes (which I believe Eric is already eliminating), I have no additional feedback. Hope to be a more useful reviewer in future.

@eweitz eweitz merged commit 7addbe4 into master Oct 30, 2019
@eweitz eweitz deleted the ew-remove-firestore-emulator branch November 20, 2019 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants