test: audit + restructure tests

[jsstevenson]

close #239
close #134

• Add more storage tests. Try to cover stuff that we've added.
• Fix interdependence between testing modules. No more need to guarantee an order. In fact, I think individual cases (functions) are no longer interdependent although I might've missed something.
• Organizational changes to tests, trying to add a bit more structure
• Consolidate various variation fixtures into variations.json so that they can be reused easily across modules. Try to standardize tertiary properties that get used (e.g. params when testing registration), although I think these are usually better added at the test module level (see changes in test_liftover.py). Add a basic Pydantic check on their way in to ensure that nothing's breaking.
• Add tests CI action. This is currently limited to just a single case to prove the concept. (Wait duh this actually covers a lot of stuff -- everything in tests/unit except liftover which I'm working on a fix for). The challenge is that we have a lot of architectural inter-dependencies between things and that's harder to unit test w/o mocks.

[jennifer-bowser]

I have a couple of minor tweaks and questions, but overall this looks awesome!! You've made some really good improvements to our testing.

[jennifer-bowser]

Everything looks great, with the one exception of this test that's failing because the string that's being compared is getting extra quotation marks inserted somewhere:

FAILED tests/unit/storage/test_postgres.py::test_annotations_crud - assert '"pathogenic"' == 'pathogenic'

EDIT: Here's the more detailed error message:

>       assert result[0].annotation_value == ann1.annotation_value
E       assert '"pathogenic"' == 'pathogenic'
E         
E         - pathogenic
E         + "pathogenic"
E         ? +          +

tests/unit/storage/test_postgres.py:282: AssertionError

[jsstevenson]

@jennifer-bowser interesting! That test is actually passing in CI and I'm not able to replicate this locally either

[jsstevenson]

stuff I'd be curious about

• whether there's a different outcome from b81b48e vs 1b14d53
• whether there's a different outcome from current dependency installs (pip freeze > old-dependencies.txt) vs deleting and recreating the venv
• whether this is a postgres version issue (psql -U postgres -c 'SELECT version();' ) -- I'm on 14.18 locally, installed with homebrew

[korikuzma]

I just setup a new anyvar environment and all tests pass on my machine

[jennifer-bowser]

AHA! The culprit appears to be that I was using postgres version 17.5, and you guys are using 14.1*. I downgraded my test DB version and the test now passes!

(EDIT: I thought the culprit was my postgres version, but apparently that's not the issue after all?? Somehow downgrading my postgres version fixed the issue for me, but CI can run the tests on 17.5 without any problems soooo 🤷🏻‍♀️ )

A few thoughts, though:

1. Our compose.yaml file specifies postgres version 17. Since that's not compatible with our setup, I think we should downgrade that in our compose file (or upgrade everything else in our codebase to be compatible with version 17, but that would be out of scope for this ticket) -> Nevermind, since this doesn't actually seem to be the problem, we can disregard this.

2. Separately, I'm getting a different error on another test:

FAILED tests/integration/vcf/test_annotate_vcf.py::test_vcf_registration_async - 
    AssertionError: {"error":"An existing run with id 12345 is SUCCESS. Fetch the completed run 
    result before submitting with the same run_id.","error_code":null}

On my feature branch for #253, I've added this code to test_vcf_registration_async, and that fixed the problem for me. Though it's odd if I'm the only one running into this issue.

Ok never mind on this too I guess? For some reason I'm only running into this issue some of the time, it's not reliably reproducible for me. Maybe just leave this be for now, it seems my computer is just cursed 😭

[jsstevenson]

I thought the culprit was my postgres version, but apparently that's not the issue after all?? Somehow downgrading my postgres version fixed the issue for me, but CI can run the tests on 17.5 without any problems soooo

Yeah I am not sure how to proceed here -- hopefully we feel okay as long as GitHub Actions uses the same image as the docker-compose file? I also tried a few other images, and went ahead and upgraded my local brew instance to v18 just to be safe.

Separately, I'm getting a different error on another test

Great find. Yeah, this one had passed for me once so I didn't look too hard, but I'm also seeing a few sporadic instances of failures. I refactored the change you'd made as a setup/teardown fixture following the pattern recommended here and now everything seems to be passing consistently.

[jsstevenson]

yolo