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

Check tests work on master, and assess PR: 68 #70

Closed
agstephens opened this issue Apr 14, 2021 · 3 comments
Closed

Check tests work on master, and assess PR: 68 #70

agstephens opened this issue Apr 14, 2021 · 3 comments
Assignees

Comments

@agstephens
Copy link
Contributor

agstephens commented Apr 14, 2021

Look into test failures and accept PR if we get it all working.

#68

You can see from the PR, some of the unit tests are failing when running as GitHub Actions (i.e. continuous integration on github). You can click and review the details to see which tests are failing. It would be worth checking out master first and running the tests on that to check whether there is a difference in the two branches - or whether the tests are failing on both.

@alaniwi
Copy link
Collaborator

alaniwi commented Apr 19, 2021

What I tried:

on an Ubuntu box:

Testing ESGF/master

  • install as follows:
    • create and activate a Python 3 virtual env, clone the repo, then in master do pip install --ugrade pip, pip install -r requirements.txt, pip install .
  • test using: cd tests and py.test
  • many tests fail, for example:
============================================= FAILURES =============================================
_________________________________ TestConnection.test_blank_query __________________________________
Traceback (most recent call last):
  File "/path/to/esgf-pyclient/tests/test_connection.py", line 24, in test_blank_query
    json = conn.send_search({})
  File "/tmp/venv/lib/python3.8/site-packages/pyesgf/search/connection.py", line 156, in send_search
    self.open()
  File "/tmp/venv/lib/python3.8/site-packages/pyesgf/search/connection.py", line 98, in open
    self._passed_session, requests_cache.core.CachedSession)):
AttributeError: module 'requests_cache' has no attribute 'core'
_____________________________ TestConnection.test_connection_instance ______________________________
Traceback (most recent call last):
  File "/path/to/esgf-pyclient/tests/test_connection.py", line 80, in test_connection_instance
    session = requests_cache.core.CachedSession(self.cache,
AttributeError: module 'requests_cache' has no attribute 'core'

(a path munged above for privacy)

Overall: 35 failed, 13 passed, 12 skipped, 1 warning in 1.06s

Testing the larsbuntemeyer/requests_cache branch

  • Then in the clone, add the remote https://github.com/larsbuntemeyer/esgf-pyclient . Check out the requests_cache from this remote and rerun the installation and the tests

  • This time: most pass: 2 failed, 46 passed, 12 skipped, 1 warning in 336.30s (0:05:36)

Remaining failures are in: TestConnection.test_get_shard_list and TestResults.test_batch_size_has_no_impact_on_results. (There was also a failure in TestResults.test_shards_constrain4 due to timeout from esgf-node.llnl.gov, which was not repeatable when rerun.)

Both of these remaining failures were included in the 35 tests that were failing with module 'requests_cache' has no attribute 'core' in master (i.e. tests which were not being run properly in master).

Of these:

  • The failure in TestConnection.test_get_shard_list is an artefact of a previous wrong change - in this commit, line 46 of tests/test_connection.py (change is not valid because replica shards at CEDA live on esgf-index2, hence only one shard on esgf-index1 and the assertion fails). We can separately revert this line. Failing at:
  File "/path/to/esgf-pyclient/tests/test_connection.py", line 46, in test_get_shard_list
    assert len(shards['esgf-index1.ceda.ac.uk']) > 1
AssertionError: assert 1 > 1
  • The failure in test_batch_size_has_no_impact_on_results may be a genuine failure that we still need to fix. At least it is being tested properly now. Fails at:
Traceback (most recent call last):
  File "/path/to/esgf-pyclient/tests/test_results.py", line 256, in test_batch_size_has_no_impact_on_results
    assert len(ids_batch_size_50) == len(ids_batch_size_100)
AssertionError: assert 50 == 100

Conclusion

Recommend merge (and also revert Carsten's change to line 46 of tests/test_connection.py)

@agstephens
Copy link
Contributor Author

Leaving open until tests have been addressed, tagged: @alaniwi

@valeriupredoi
Copy link
Collaborator

the install and test procedure is now changed to the modern conda+mamba env creation, pip install and pytest:

  • mamba env create -n esgf-pyclient -f environment.yml
  • pip install -e .[develop]
  • pytest

There are number of tests that fail from test_results.py module, and I have #98 open for that and a number of transitory fails due to ESGF servers' 503's that I opened #99 for, all other tests pass, and this can now be closed 👍

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

No branches or pull requests

3 participants