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

Try setting up CI #142

Merged
merged 28 commits into from
Nov 14, 2023
Merged

Try setting up CI #142

merged 28 commits into from
Nov 14, 2023

Conversation

sampsyo
Copy link
Collaborator

@sampsyo sampsyo commented Sep 24, 2023

A totally not ready, experimental approach to #53. Nothing to see here yet.

@sampsyo
Copy link
Collaborator Author

sampsyo commented Oct 7, 2023

One note as this starts to take shape: so far, this only runs the tests for slow-odgi. It should eventually also run the tests for the data generator and the new tests for the parser added in #136.

@sampsyo sampsyo marked this pull request as ready for review November 5, 2023 19:31
@sampsyo
Copy link
Collaborator Author

sampsyo commented Nov 6, 2023

I think this is ready to go (i.e., we can merge if @anshumanmohan & @susan-garry approve)! The point is to make sure our tests that are currently pass keep passing, and we don't accidentally break some tests without noticing (which has happened a few times!). It will also hopefully give us some confidence to do some refactoring, and even changing the test setup, because we'll get feedback if we do anything wrong.

This CI setup tests only two things: slow_odgi (using odgi itself as the differential testing basis) and the Pollen DSL parser (which has RON snapshots of parsed ASTs checked in). It does not cover pollen_data_gen (whose testing setup relies on a baseline that uses odgi's Python bindings, our use of which is going away soon anyway, so it didn't seem worth it).

The only tricky part of this CI setup was running odgi, without compiling it from source (which would be real slow). What I ended up doing was using a preexisting Docker container for odgi. So whenever our tests would invoke a real odgi binary, they instead do docker run odgi.

GFA_URL := https://raw.githubusercontent.com/pangenome/odgi/ebc493f2622f49f1e67c63c1935d68967cd16d85/test
GFA_ZIP_URL := https://s3-us-west-2.amazonaws.com/human-pangenomics/pangenomes/scratch/2021_05_06_pggb/gfas/chr8.pan.gfa.gz

# A smaller set of test inputs for faster runs.
Copy link
Contributor

Choose a reason for hiding this comment

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

🤩

branches:
- main

jobs:
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, this feels a lot like our Dockerfile. Which is a good thing. In fact maybe in the future the Dockerfile should learn from your system of sourcing and running odgi.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, totally. I stole some of the stuff directly from the Dockerfile. Someday merging these things could be cool, but I have never really figured out the way to do this in general for other projects (i.e., to use a "base" Dockerfile twice, once for the "real" Dockerfile and once for the CI).

Copy link
Contributor

@anshumanmohan anshumanmohan left a comment

Choose a reason for hiding this comment

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

Looks great, Adrian. Thank you once again for figuring this out!

@sampsyo
Copy link
Collaborator Author

sampsyo commented Nov 14, 2023

Excellent; merging this now!

@sampsyo sampsyo merged commit 43b24bc into main Nov 14, 2023
3 checks passed
@sampsyo sampsyo deleted the ci branch November 14, 2023 20:48
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.

2 participants