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

Clean up test folder #78

Open
HEdingfield opened this issue Jul 4, 2018 · 4 comments · Fixed by #143
Open

Clean up test folder #78

HEdingfield opened this issue Jul 4, 2018 · 4 comments · Fixed by #143

Comments

@HEdingfield
Copy link
Contributor

HEdingfield commented Jul 4, 2018

It would be nice to standardize the file names and clean up the test folder. I like the format date-location-office. Maybe we could consistently prepend cvr_* and config_* to those.

There also might be some test files in there that aren't needed anymore or, if they are, could also have more consistent names (e.g. "minimum-threshold-test-cvr.xlsx", "test_continue_tabulation_config.json", etc.).

@tarheel tarheel removed their assignment Jul 19, 2018
@CalebKleppner CalebKleppner added this to the v 0.9 Cleanup milestone Aug 9, 2018
HEdingfield added a commit that referenced this issue Aug 30, 2018
…nfig fields inside, rearranged fields so they match ordering in docs and config creator GUI, regenerated test_data config files so match new format; fixed #78: standardized test_data CVR and config file names.
@HEdingfield
Copy link
Contributor Author

Re-opening because it seems like tests added since this was completed don't follow these standards. In particular, should address:

  • Everything that's run in TabulatorTests.java should follow the consistent test_* format (I see lots of folders with *_test and some that don't even have the word test.
  • It seems like some CVR files, like 2013_minneapolis_park_cvr.xlsx are used multiple times (e.g. 2013_minneapolis_park_hare_cvr.xlsx, 2013_minneapolis_park_bottoms_up_cvr.xlsx, 2013_minneapolis_park_sequential_cvr.xlsx all appear to be the exact same file; @tarheel please confirm); maybe we should have a single copy and each of those test configs reference the same location?
  • This is a minor / partial blocker to Decide if Gradle distribution should include test files and docs #281, because I think we need to distinguish between files that are included as regression tests, and those that are there for manual testing / playing with by users. Perhaps the latter type should be stored in a separate directory.
  • Which leads to a final thing we should add: a setup that lets users easily trigger / play with the interactive tiebreaker.

@tarheel
Copy link
Contributor

tarheel commented Jun 21, 2019

Yes, we do have a number of duplicate CVR files in the test data. If we wanted to avoid that, we would need to create an exception to the naming scheme, I guess. Also, 2013_minneapolis_mayor_scale has 13 copies of the same file because we don't allow a config to list the same source file multiple times. I agree it's dumb to have duplicate files in the repo, though. Feel free to address these issues in a way that you think is reasonable.

@moldover
Copy link
Contributor

It's not worth it to come up with a scheme to exclude a mere 25MB of test files. If we make a distro which includes test files, we should include them all.

@HEdingfield
Copy link
Contributor Author

Began the process of distinguishing between "test" and "sample" input files in PR #316. Verified that, in that PR, only the sample_interactive_tiebreak folder isn't used in a unit test, so all other folders should start with the prefix "test".

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

Successfully merging a pull request may close this issue.

4 participants