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

Skip NNDC/XCOM tests if databases are down #333

Merged
merged 25 commits into from May 16, 2022
Merged

Skip NNDC/XCOM tests if databases are down #333

merged 25 commits into from May 16, 2022

Conversation

jvavrek
Copy link
Contributor

@jvavrek jvavrek commented Apr 1, 2022

Closes #332

@jvavrek jvavrek requested a review from jccurtis April 1, 2022 00:38
@jvavrek
Copy link
Contributor Author

jvavrek commented Apr 1, 2022

@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
Copy link
Collaborator

jccurtis commented Apr 4, 2022

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

@markbandstra
Copy link
Member

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
Copy link
Contributor Author

jvavrek commented Apr 4, 2022

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?

@jvavrek jvavrek changed the title Increase number of flaky reruns and add 2 s delay Parallelize tests and skip NNDC/XCOM tests if databases are down Apr 4, 2022
@jvavrek jvavrek marked this pull request as draft April 4, 2022 23:14
@jvavrek jvavrek self-assigned this Apr 4, 2022
@markbandstra
Copy link
Member

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
Copy link
Contributor Author

jvavrek commented Apr 5, 2022

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

@jvavrek jvavrek changed the title Parallelize tests and skip NNDC/XCOM tests if databases are down Skip NNDC/XCOM tests if databases are down Apr 5, 2022
@jvavrek jvavrek marked this pull request as ready for review April 5, 2022 02:46
@jvavrek
Copy link
Contributor Author

jvavrek commented Apr 5, 2022

@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!

@jvavrek jvavrek merged commit 574a460 into main May 16, 2022
@jvavrek jvavrek deleted the patch-flaky-tests branch May 16, 2022 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flaky NNDC tests failing
4 participants