Skip to content

179 chart tests #180

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 24 commits into from
Mar 22, 2020
Merged

179 chart tests #180

merged 24 commits into from
Mar 22, 2020

Conversation

pjhoberman
Copy link
Collaborator

Also includes creating a tests directory and moving test_app.py there

PJ Hoberman added 3 commits March 21, 2020 13:47
#179 chart tests
eff8a7d 
 - Added test_admitted_patients_chart
 - Moved projection_admits and census_df to csv files to avoid using constants from settings.py or app.py
#179 moved tests to test package
45b9d20
#179 importing alt directly rather than from app.py
ef67a40
@pjhoberman pjhoberman linked an issue Mar 21, 2020 that may be closed by this pull request
Tests around charts #179
Closed
2 tasks
@jlubken
Copy link
Collaborator

jlubken commented Mar 21, 2020

Some thoughts for the future.

Usually tests is treated as a separate module so I resides next to src, not inside src. However this also means that penn_chime is an install-able module and has a setup.py file too. The separate tests module ensures that what is tested is what is actually installed in site-packages, not what a developer accidentally has available in their local directory. Furthermore, the module installation pip install . must be added to the Dockerfile and other build/deploy steps.

@pjhoberman
Copy link
Collaborator Author

Done! Moved to its own module.

quinn-dougherty and others added 19 commits March 21, 2020 18:07
@quinn-dougherty
refactored functions so they take a parameters instance instead of a …
9921c26 
…bucket of args
@quinn-dougherty
moved build_*_df funcs into models.py since they have mathematical co…
44bc837 
…ntent
@quinn-dougherty
fixing lint
e706e83
@quinn-dougherty
getting tests to work
f68257a
@quinn-dougherty
fixing tests
2a40b7a
@quinn-dougherty
ffixing tests
9947fe5
@quinn-dougherty
improved lint situation
9d49381
@quinn-dougherty
fixing tests
865736f
@quinn-dougherty
changed projections_admit to admissions_df
9f74c28
@quinn-dougherty
fixing import in test for admissions_df
191d7a6
@quinn-dougherty
fixed test from renaming admissions_df
7068329
@jlubken
Merge pull request #182 from CodeForPhilly/112-model
6af5522 
112 model
#179 chart tests
f39b057 
 - Added test_admitted_patients_chart
 - Moved projection_admits and census_df to csv files to avoid using constants from settings.py or app.py
#179 moved tests to test package
ea95104
#179 importing alt directly rather than from app.py
7b9ac46
#179 moved tests to its own module
e3788f5
chose the wrong imports on final rebase conflict
4b493a2
#179 Updated tests to match new chart and param functions
d615e56
fixing merge conflicts
69469fe
quinn-dougherty
quinn-dougherty approved these changes Mar 22, 2020
View reviewed changes
Copy link
Collaborator

@quinn-dougherty quinn-dougherty left a comment

Choose a reason for hiding this comment

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

thanks PJ!

@quinn-dougherty quinn-dougherty merged commit c826250 into develop Mar 22, 2020
@quinn-dougherty quinn-dougherty deleted the 179-chart-tests branch March 22, 2020 01:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@quinn-dougherty quinn-dougherty quinn-dougherty approved these changes

@sam-writer sam-writer Awaiting requested review from sam-writer

@jlubken jlubken Awaiting requested review from jlubken

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Tests around charts
3 participants
@pjhoberman @jlubken @quinn-dougherty