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

Update GH Action tests and CodeCov #99

Merged
merged 33 commits into from
Apr 12, 2024
Merged

Conversation

rickecon
Copy link
Member

@rickecon rickecon commented Apr 11, 2024

This PR:

  • Adds a list of file change event triggers to build_and_test.yml so that those tests only run when one of those files is changed.
  • Updates the codecov GH Action to version 4 and adds a secret token.
  • Adds a list of file change event triggers to deploy_docs.yml and docs_check.yml, and limits docs_check.yml to only run on pull requests.
  • Fixes a small typo in tax_functions.md in order to test if the event triggers worked properly (yes, they worked)
  • Updated some dependencies in environment.yml.
  • Updated three data files in the /tests/test_io_data/ file that used output from the taxcalc package. This package was recently updated. I also changed the test_get_data() test in the test_get_micro_data.py file because the new taxcalc data included four years instead of two years. In order to conserve repo memory footprint, we deleted the last two years of the output.

cc: @jdebacker

@rickecon rickecon changed the title Update GH Action tests Update GH Action tests and CodeCov Apr 11, 2024
@rickecon
Copy link
Member Author

@jdebacker. These three test failures in the last three tests of the test_get_micro_data.py Linux Python 3.9 tests seem like legitimate failures.

tests/test_get_micro_data.py ........FFF

@rickecon
Copy link
Member Author

I tried adding the windows-latest OS tests back into build_and_test.yml, but I got the same error as noted in Issue #78. I am removing the Windows OS tests again from build_and_test.yml.

Run pytest -m 'not local and not regression' --cov=./ --cov-report=xml
  pytest -m 'not local and not regression' --cov=./ --cov-report=xml
  shell: C:\Program Files\Git\bin\bash.EXE -l {0}
  env:
    INPUT_RUN_POST: true
    CONDA: C:\Users\runneradmin\miniconda[3](https://github.com/PSLmodels/OG-USA/actions/runs/8643695604/job/23698134190?pr=99#step:5:3)
    CONDA_PKGS_DIR: C:\Users\runneradmin\conda_pkgs_dir
C:\Users\runneradmin\miniconda3\envs\ogusa-dev\lib\site-packages\pep8.py:110: FutureWarning: Possible nested set at position 1
  EXTRANEOUS_WHITESPACE_REGEX = re.compile(r'[[({] | []}),;:]')
WindowsError

@@ -1,7 +1,5 @@
name: Build and test [Python 3.9, 3.10]

name: Build and test
on: [push, pull_request]
Copy link
Member

Choose a reason for hiding this comment

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

Re my comments a few weeks ago about unnecessary compute time, from a read of the updated workflow docs, I think we should be able to condition tests to run only when relevant files are affected.

E.g., something like:

on:
  [push, pull_request]:
    paths:
      - './ogusa/**.py'
      -  './tests/**.py'

Copy link
Member Author

Choose a reason for hiding this comment

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

@jdebacker. Yes. I'll add this. But we probably need to expand beyond the .py files.

  • In the ./ogusa/ directory, we have all .py files except for one .json file. I think we need to add the .json file to the directory tracking.

  • In the ./tests/ directory, we probably also need to test changes in the .pkl files and .csv files in the ./tests/test_io_data/ directory.

  • In the main directory, we probably need to run the tests whenever environment.yml, pyproject.toml, pytest.ini, or setup.py changes.

@rickecon rickecon marked this pull request as draft April 11, 2024 20:22
@rickecon rickecon marked this pull request as ready for review April 12, 2024 05:19
@codecov-commenter
Copy link

codecov-commenter commented Apr 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.70%. Comparing base (9f1d02f) to head (415df86).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #99      +/-   ##
==========================================
+ Coverage   70.32%   78.70%   +8.37%     
==========================================
  Files          22       11      -11     
  Lines        1365      850     -515     
==========================================
- Hits          960      669     -291     
+ Misses        405      181     -224     
Flag Coverage Δ
unittests 78.70% <ø> (+8.37%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 11 files with indirect coverage changes

@jdebacker
Copy link
Member

LGTM, thanks @rickecon! Ready for this to be merged?

@rickecon
Copy link
Member Author

@jdebacker. Let me do one last check of a couple things. I still have this white whale of trying to get the Windows tests working again.

@rickecon
Copy link
Member Author

@jdebacker. This PR is ready for your final review. I have made all the changes I am going to make. I spent hours trying to get the windows-latest tests added back in. But I got everything else in. All these tests should pass by 5pm ET.

@rickecon
Copy link
Member Author

@jdebacker. All tests passed locally on my machine.

(ogusa-dev) richardevans@Richards-MacBook-Pro-2 OG-USA % pytest
======================= test session starts =========================
platform darwin -- Python 3.11.8, pytest-8.1.1, pluggy-1.4.0
rootdir: /Users/richardevans/Docs/Economics/OSE/OG-USA
configfile: pytest.ini
testpaths: ./tests
plugins: cov-5.0.0, xdist-3.5.0
collected 37 items                                                                                                        

tests/test_calibrate.py ....                                  [ 10%]
tests/test_get_micro_data.py ..............                   [ 48%]
tests/test_income.py ............                             [ 81%]
tests/test_psid_data_setup.py ...                             [ 89%]
tests/test_run_example.py .                                   [ 91%]
tests/test_utils.py .                                         [ 94%]
tests/test_wealth.py ..                                       [100%]
========== 37 passed, 111 warnings in 767.83s (0:12:47) ==================

@jdebacker jdebacker merged commit 4c0e3d8 into PSLmodels:master Apr 12, 2024
6 checks passed
@rickecon rickecon deleted the tests branch April 13, 2024 07:18
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.

3 participants