Fix skipping tests in test_against_taxsim.py

ss7886 commented 1 year ago

Update fiscalsim_us/tests/microsimulation/test_against_taxsim.py to stop skipping/failing tests on Windows (still needs testing for Mac and Linux).

rickecon commented 1 year ago

@ss7886. I fixed the issue with my terminal giving me the following error when I ran the make test command.

error xcrun: error: invalid active developer path (/Library/Developer/CommandLineTools), missing xcrun at: /Library/Developer/CommandLineTools/usr/bin/xcrun

Something must have changed with the Xcode developer tools app on my Mac when I upgraded to the Ventura OS. I just had to reinstall Xcode. Now it all works.

rickecon commented 1 year ago

@ss7886. This looks great. I pulled your branch for this PR and ran the make test command. For the pytest fiscalsim_us/tests/ --maxfail=0 portion, I get the following output.

platform darwin -- Python 3.9.13, pytest-5.4.3, py-1.11.0, pluggy-0.13.1
rootdir: /Users/richardevans/Docs/Economics/OSE/fiscalsim-us
plugins: dependency-0.5.1
collected 1072 items                                                                                                                                            

fiscalsim_us/tests/test_variables.py .................................................................................................................... [ 10%]
......................................................................................................................................................... [ 25%]
......................................................................................................................................................... [ 39%]
......................................................................................................................................................... [ 53%]
......................................................................................................................................................... [ 67%]
......................................................................................................................................................... [ 82%]
......................................................................................................................................................... [ 96%]
..........................                                                                                                                                [ 98%]
fiscalsim_us/tests/microsimulation/test_against_taxsim.py .....                                                                                           [ 99%]
fiscalsim_us/tests/microsimulation/test_microsim.py .                                                                                                     [ 99%]
fiscalsim_us/tests/microsimulation/data/test_imports.py ..                                                                                                [ 99%]
fiscalsim_us/tests/microsimulation/data/acs/test_acs.py ss                                                                                                [ 99%]
fiscalsim_us/tests/microsimulation/data/cps/test_cps.py ..                                                                                                [100%]

Note that both of my test_cps.py tests pass, whereas you have one failing. Are you running these tests within the fiscalsim-us conda environment? That might be the difference. Once you confirm that you get the same test results as what I showed above by running make test in the fiscalsim-us conda environment, I am ready to merge this.

rickecon commented 1 year ago

@ss7886. Am I remembering correctly that you have a Windows machine?

@mitchellpound. Will you please run these fiscalsim-us tests on your Linux machine while in your fiscalsim-us conda environment to make sure you get the following output when you run the make test command from the terminal?

You'll need to do the following steps before you run the tests.

Make sure you have the most recent version of the repo in your main branch of your fork.
Make sure you have the most recent version of the fiscalsim-us conda environment
Create a new branch and pull @ss7886's PR branch to it. Make sure you are in Sam's PR branch on your machine when you test this. The commands are below:
```
git checkout -b ss7886-main main
git pull https://github.com/ss7886/fiscalsim-us.git main
```
You can either run make test in your terminal, or you can run the following two tests manually/individually.

For the coverage test coverage run -a --branch -m policyengine_core.scripts.policyengine_command test fiscalsim_us/tests/policy/ -c fiscalsim_us you get:

=== 1127 passed in 50.87s ===

For the pytest tests pytest fiscalsim_us/tests/ --maxfail=0, you get:

platform darwin -- Python 3.9.13, pytest-5.4.3, py-1.11.0, pluggy-0.13.1
rootdir: /Users/richardevans/Docs/Economics/OSE/fiscalsim-us
plugins: dependency-0.5.1
collected 1072 items                                                                                                                                            

fiscalsim_us/tests/test_variables.py .................................................................................................................... [ 10%]
......................................................................................................................................................... [ 25%]
......................................................................................................................................................... [ 39%]
......................................................................................................................................................... [ 53%]
......................................................................................................................................................... [ 67%]
......................................................................................................................................................... [ 82%]
......................................................................................................................................................... [ 96%]
..........................                                                                                                                                [ 98%]
fiscalsim_us/tests/microsimulation/test_against_taxsim.py .....                                                                                           [ 99%]
fiscalsim_us/tests/microsimulation/test_microsim.py .                                                                                                     [ 99%]
fiscalsim_us/tests/microsimulation/data/test_imports.py ..                                                                                                [ 99%]
fiscalsim_us/tests/microsimulation/data/acs/test_acs.py ss                                                                                                [ 99%]
fiscalsim_us/tests/microsimulation/data/cps/test_cps.py ..                                                                                                [100%]
...
=== 1070 passed, 2 skipped, 32 warnings in 35.82s ===

rickecon commented 1 year ago

@ss7886. I closed my PR in the policyengine-us repository. I tried every permutation here of python=3.9, python>=3.8, python=3.8, tables=3.6.1 and tables. Fixing this to allow for the GitHub Action CI for test_against_taxsim.py will probably require some changes in the policyengine-core repository.

As Sam found dug out, he found that these test failures and errors likely stem from an incompatibility between the pandas.HDFStore() method and the version of the tables package. The pd.HDFStore method is used six times in the policyengine-core/data/dataset.py module in the load(), save(), and keys() functions. This incompatibility is not picked up in the conda environment creation or pip package compatibility checks.

I am backing out of this one until and unless Sam makes some more progress.

TheCGO / fiscalsim-us

Fix skipping tests in test_against_taxsim.py #21