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

add an optional test to make sure full validate produce is run when l… #685

Merged
merged 34 commits into from
Sep 6, 2024

Conversation

sierra-moxon
Copy link
Member

@sierra-moxon sierra-moxon commented Aug 6, 2024

The first iteration of this PR had methods to download, store, use, and then clean up metadata from the go-site repo during the test execution, but it involved a lot of file wrangling that ended up being overly complicated and hard to maintain. I decided to just copy over the basic metadata into the resources/metadata directories in this PR and use it statically. This lets me run the full 'validate.produce' command as an automated test.

It also adds the download and maintenance of go-basic.obo as a pytest fixture. Since this is just one file, and since it gets it from snapshot that can sometimes be unavailable, it felt like a worthwhile resource to grab and maintain dynamically.

Finally, it adds some logic to implementations that use poetry so that this test gets executed locally
(e.g. I will run it always by default locally) but doesn't run as part of CI (because it takes quite a bit of time to run). There are other tests that execute parts and pieces of the validate.produce command, but there are several cases I want to test that require the full step here to run (e.g. does the final GAF produced have isoforms as subjects or not).

This all just saves me from forgetting to run this full test of the validate.produce command manually as underlying routines change.

@sierra-moxon sierra-moxon requested review from dustine32 and kltm August 12, 2024 21:47
@sierra-moxon sierra-moxon marked this pull request as ready for review August 12, 2024 21:50
Copy link
Member

@kltm kltm left a comment

Choose a reason for hiding this comment

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

@sierra-moxon This looks like a good set of ease-of-use and separation changes. I'm fine with this being merged as-is.

One thing that I might be interested in, from past experience, would be a README somewhere that explains how to update the metadata (and the rationale for making things static), in case things change enough to warrant updates.

@dustine32
Copy link
Collaborator

@sierra-moxon I'm getting these errors when attempting make travis_test_full, though I'm still using the antiquated venv environment so maybe this doesn't happen with poetry:

$ .env/bin/activate
(env)$ python3 --version
Python 3.10.3
(env)$ make travis_test_full
...
=============================================== ERRORS ================================================
______________________ ERROR at setup of test_validate_resulting_gaf[gaf_setup0] ______________________
ScopeMismatch: You tried to access the function scoped fixture runner with a session scoped request object, involved factories:
tests/test_validate_cli.py:47:  def gaf_setup(request, runner, go_json)
tests/test_validate_cli.py:15:  def runner()
______________________ ERROR at setup of test_validate_resulting_gaf[gaf_setup1] ______________________
ScopeMismatch: You tried to access the function scoped fixture runner with a session scoped request object, involved factories:
tests/test_validate_cli.py:47:  def gaf_setup(request, runner, go_json)
tests/test_validate_cli.py:15:  def runner()
______________________ ERROR at setup of test_validate_resulting_gaf[gaf_setup2] ______________________
ScopeMismatch: You tried to access the function scoped fixture runner with a session scoped request object, involved factories:
tests/test_validate_cli.py:47:  def gaf_setup(request, runner, go_json)
tests/test_validate_cli.py:15:  def runner()
______________________ ERROR at setup of test_validate_resulting_gaf[gaf_setup3] ______________________
ScopeMismatch: You tried to access the function scoped fixture runner with a session scoped request object, involved factories:
tests/test_validate_cli.py:47:  def gaf_setup(request, runner, go_json)
tests/test_validate_cli.py:15:  def runner()

@dustine32
Copy link
Collaborator

@sierra-moxon Actually, these tests are running in poetry since I see this at the top of the output:

Running tests in Poetry environment...

Guessing it's because I have both a venv folder and pyproject.toml file to meet this criteria:

@if [ -d ".venv" ] && [ -f "pyproject.toml" ]; then \

I'll keep debugging these errors...

@sierra-moxon
Copy link
Member Author

Hi @dustine32 - thank you so much for testing the travis_test_full target. Would you mind try pulling and running the test again (I fixed the fixture failure you found, but I'd like to make sure there aren't any venv issues)?

You can configure the slow test datasets in ontobio/tests/test_validate_cli.py by modifying datasets_to_test list if you just want to see that it runs for you (the fastest are goa-cow and tair; noting that the available metadata is for: mgi, cdg, tair, goa-cow|chicken, zfin at the moment, so any of those should work as test datasets).

I have not been able to recreate the snapshot pipeline error that we found in this ticket with this branch: #689 (I believe it was taken care of in a previous PR -- the solution was/is to GPI files before and after the isoform check).

Copy link
Collaborator

@dustine32 dustine32 left a comment

Choose a reason for hiding this comment

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

Thanks @sierra-moxon! The make travis_test_full now passes (btw took about 5min for me).

@sierra-moxon sierra-moxon merged commit ac4ea1a into master Sep 6, 2024
2 checks passed
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