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

Improve testing #2021

Draft
wants to merge 16 commits into
base: master
Choose a base branch
from
Draft

Improve testing #2021

wants to merge 16 commits into from

Conversation

anth-volk
Copy link
Collaborator

Fixes #2015
Partially fixes #2013 (some tests will need to be broken up into relevant files)
Fixes #2007
Fixes #1357

Inspired by #2010 (a regression introduced by #2006), this PR creates a new testing structure for economy-wide simulations. The aim is to make it simple to test that a series of common reform impacts (e.g., modifying the federal income tax rate) still runs and returns values within reason for certain key measures. This does not compare outputs with country packages (that's open as #2017, but it's unclear if its likely slow pace would justify writing it).

This also begins to move toward a more concise testing folder structure, whereby integration tests (any that hit use the rest_client Pytest fixture to simulate an endpoint hit, then check its response) are distinct from unit tests. A few tests are either also being modified in an open PR or would need to be subdivided to match the internal file structure, so they've been relegated to a temporary to_be_refactored folder.

@anth-volk anth-volk force-pushed the fix/improved-testing branch from 3955af4 to b9ab0e7 Compare November 26, 2024 00:03
@anth-volk anth-volk marked this pull request as draft November 26, 2024 20:57
@anth-volk anth-volk marked this pull request as ready for review November 27, 2024 00:10
Copy link
Collaborator

@nikhilwoodruff nikhilwoodruff 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 like an improvement to me. But could we at least try using the subsample feature to reduce the test runtimes? We've jumped quite a bit to 15m, and I don't think it would reduce accuracy if we're just testing large bounds.

@anth-volk
Copy link
Collaborator Author

@nikhilwoodruff My understanding is that we already do: the make test command passes MAX_HOUSEHOLDS=10000 as an environment variable before running the tests

@anth-volk anth-volk changed the title Fix/improved testing Improve testing Nov 27, 2024
@anth-volk
Copy link
Collaborator Author

Per discussion with @nikhilwoodruff, I'll be cutting a few tests out of this later today. Also need to rebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants