lbl-anp / becquerel

Becquerel is a Python package for analyzing nuclear spectroscopic measurements.
Other
43 stars 16 forks source link

Skip NNDC/XCOM tests if databases are down #333

Closed jvavrek closed 2 years ago

jvavrek commented 2 years ago

Closes #332

jvavrek commented 2 years ago

@jccurtis I feel like my increasing the retries and adding a delay is ultimately a bad idea... this means up to 20 s per test, and if NNDC is down as it was in the last pipeline, the whole test suite can take an hour and a half to run!

jccurtis commented 2 years ago

@jvavrek can we change the tests to use xdist and run in parallel?

markbandstra commented 2 years ago

Maybe we should add a simple test (a requests POST?) and see if it is successful. If not, we can disable the NNDC tests. We also might have to do this for XCOM.

jvavrek commented 2 years ago

With pytest-xdist we get some strange cache errors:

______________________ TestCacheExceptions.test_bad_path _______________________
[gw0] linux -- Python 3.6.15 /opt/hostedtoolcache/Python/3.6.15/x64/bin/python

self = <df_cache_test.TestCacheExceptions object at 0x7f084ab[57](https://github.com/lbl-anp/becquerel/runs/5825240685?check_suite_focus=true#step:7:57)9b0>

    def test_bad_path(self):
        """Test ExampleCache.check_path() exception for a bad path."""
        d = ExampleCache()
        d.path = "/bad/path"
        with pytest.raises(CacheError):
            d.check_path()
        with pytest.raises(CacheError):
>           d.check_file()
E           Failed: DID NOT RAISE <class 'becquerel.tools.df_cache.CacheError'>

tests/df_cache_test.py:67: Failed

@markbandstra can you comment? Are we perhaps getting a race condition here?

markbandstra commented 2 years ago

I think that could be a race condition... it looks like setting d.path doesn't update d.filename, and it's possible that another test has created a file with the same name. I think that test could be fixed by the line

        d.filename = "/bad/path/filename.csv"

right before that test.

jvavrek commented 2 years ago

Now we have a DeadlockError: https://github.com/lbl-anp/becquerel/runs/5826003988?check_suite_focus=true

It appears in macos python 3.8, 3.9, but not 3.7

Possibly related:

jvavrek commented 2 years ago

Downgrading to pytest<=5.3.5 almost worked, and then failed for python 3.9.

jvavrek commented 2 years ago

@jvavrek can we change the tests to use xdist and run in parallel?

In short, no. I've opened #336 to track it further and removed xdist from this PR. Ready for review!