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

TST/CI: refactor test suite to use TeenyTweetyNet, fix #330 #339

Merged
merged 1 commit into from
Apr 3, 2021

Conversation

NickleDave
Copy link
Collaborator

generating test data:

  • rename tweetynet configs
  • add teenytweetynet configs
  • modify test_data_generate.py script
    • so it finds all configs
    • have it make model subdirectories in results/
  • rewrite Makefile
    • differentiate 'all' and 'ci' generated test data
    • change commands, variables, urls to data

refactoring tests:

  • add model parameters to fixtures:
    • to specific_config fixture
    • to specific_config_toml fixture
    • to dataframe fixtures
    • change previous_run_path fixture to previous_run_path_factory
      • use model as argument to _previous_run_path function
        returned by factory
  • add models command-line option for pytest
    • that will parametrize any test that specifies models fixture
      with whatever arguments are passed in at command line
  • use model fixture in tests
    • add model fixture to unit tests in test_core/ and test_cli/
    • use model fixture in a test in test_labeled_timebins
    • use previous_run_path_factory with model in
      test_cli/test_learncurve.py

CI:

  • have ci.yml download just test data generated for ci
  • have ci run pytest using command-line option models,
    and specifying only teenytweetynet for now
    (default but make it explicit anyway)

other refactoring

  • move src/scripts to tests/scripts
  • rename test_data to data_for_tests
    • so it doesn't look like a sub-package of tests to pytest

@NickleDave NickleDave force-pushed the use-teenytweetynet-in-tests branch 2 times, most recently from 2c29b8a to 54e0165 Compare March 28, 2021 15:41
generating test data:
- rename tweetynet configs
- add teenytweetynet configs
- modify test_data_generate.py script
  - so it finds all configs
  - have it make model subdirectories in results/
- rewrite Makefile
  - differentiate 'all' and 'ci' generated test data
  - change commands, variables, urls to data

refactoring tests:
- add `model` parameters to fixtures:
  - to `specific_config` fixture
  - to `specific_config_toml` fixture
  - to `dataframe` fixtures
  - change `previous_run_path` fixture to `previous_run_path_factory`
    - use `model` as argument to `_previous_run_path` function
      returned by factory
- add `models` command-line option for pytest
  + that will parametrize any test that specifies `models` fixture
    with whatever arguments are passed in at command line
- use `model` fixture in tests
  - add model fixture to unit tests in test_core/ and test_cli/
  - use `model` fixture in a test in test_labeled_timebins
  - use `previous_run_path_factory` with `model` in
    test_cli/test_learncurve.py
- also add `default_model` fixture that is used whenever a model
  name is needed by other fixtures (e.g. `specific_config`) but
  the model shouldn't actually matter
  + idea is this model should work no matter where tests are run,
    CI v. locally
  + this is a code smell to me -- if tests don't depend on model
    then why do I need a "dummy model"
  + but not obvious to me right now how to disentangle them, and I
    just want to get the damn CI working

CI:
- have ci.yml download just test data generated for ci
- have ci run pytest using command-line option models,
  and specifying only teenytweetynet for now
  (default but make it explicit anyway)
- fix the `fix_prep_csv_paths` script so that it correctly
  finds prep csv files with the new test data directory
  names and structure

other refactoring
- move src/scripts to tests/scripts
- rename test_data to data_for_tests
  + so it doesn't look like a sub-package of tests to `pytest`
@NickleDave NickleDave force-pushed the use-teenytweetynet-in-tests branch 2 times, most recently from e2849e2 to b7bddc5 Compare April 3, 2021 21:03
@NickleDave
Copy link
Collaborator Author

Going ahead and merging this even though not all runs are passing (but some are, yay!)

I believe this is do to an issue with running out of disk space, because it only happens to the ubuntu-latest runs.
See https://github.community/t/action-fails-after-20-minutes-without-log/18432/2 and
actions/runner-images#709 (comment)

I tried one of the workarounds (using apt-get clean) but that did not completely fix it.
Will raise a separate issue on next steps I think I can take to troubleshoot.

@NickleDave NickleDave merged commit 664682b into main Apr 3, 2021
@NickleDave NickleDave deleted the use-teenytweetynet-in-tests branch April 3, 2021 21:08
NickleDave added a commit that referenced this pull request Apr 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant