polaris-hub / polaris

Foster the development of impactful AI models in drug discovery.
https://polaris-hub.github.io/polaris/
Apache License 2.0
39 stars 2 forks source link

Test: Integration tests of the `PolarisHubClient` #31

Open cwognum opened 10 months ago

cwognum commented 10 months ago

Context

We currently do not test the PolarisHubClient automatically. There's two factors that limit us from writing tests right now:

  1. Without a headless authentication method, we cannot automatically run tests in Github Actions.
  2. Without a dev environment setup for the Polaris Hub, we would flood the prod database with artifacts.

Description

Once the above two limitations have been addresses in https://github.com/valence-labs/polaris/issues/30 and https://github.com/valence-labs/polaris-hub/issues/64, start writing test cases for the Polaris client. Currently, polaris/hub/client.py and polaris/hub/settings.py are omitted from the coverage. These will have to be added back.

Acceptance Criteria

Links

cwognum commented 9 months ago

Thinking about this some more, I think we should add a protected test branch in the Hub repo. We then use a Github CICD workflow, triggered upon each new hub release, to automatically sync the test branch with the main branch. The Vercel bot will then automatically create a new test deployment and a new Neon DB branch - associated with the test branch - that can be used for testing.

ping @mercuryseries @jstlaurent - Does this make sense?

jstlaurent commented 9 months ago

Thinking about this some more, I think we should add a protected test branch in the Hub repo. We then use a Github CICD workflow, triggered upon each new hub release, to automatically sync the test branch with the main branch. The Vercel bot will then automatically create a new test deployment and a new Neon DB branch - associated with the test branch - that can be used for testing.

* How to make sure that the new deployment has a new Neon branch? Do we need to delete the branch and recreate it or can we simply merge the latest changes from `main` into `test`?

* If the time between releases is long, how do we make sure that the artifacts don't build up endlessly? Maybe to start it's enough to also be able to manually trigger the CICD.

  * If we end up adding endpoints to the client that are concerned with deleting artifacts, we could also make sure that the tests clean up after themselves to reduce left-over artifacts.

* I was initially thinking of simply using a `development` branch as is used in the [Git flow](https://nvie.com/posts/a-successful-git-branching-model/) branching workflow to not introduce a new branch, but then the client is not tested against the latest, stable release, but rather against a dev version.

ping @mercuryseries @jstlaurent - Does this make sense?

Well, there's maybe a few ways to go about this. But we're talking end-to-end tests here, which tend to be heavier and longer to run. Maybe some integration tests with a mock Hub API might be a good stopgap measure in the meantime.

So, end-to-end testing with an actual Hub an and actual DB.

Option 1: Run local

You'll need some scaffold to do the following:

  1. Create a new branch from main in Neon
  2. Pull from GitHub the Hub's main branch
  3. Set the environment variables for that local Hub, including the Neon branch endpoint
  4. Start up the Hub
  5. Run your Polaris tests pointing to that local Hub
  6. Stop and remove the pulled code
  7. Remove the Neon branch

It's a pain to run on a laptop, but we could set that up in GitHub Actions CI and have it run there for PRs on the Polaris repo.

Option 2: Leverage Vercel

If we want to leverage Vercel, to get as close to prod as possible, then it would probably have to go down this way:

  1. Pull from GitHub the Hub's main branch
  2. Create a new Git branch locally, off main, and then push it to GitHub
  3. Wait for Vercel to create a Deployment Preview for the branch (complete with Neon branch)
  4. Run you Polaris tests pointing to that deployment (Might need to validate that the deployment is accessible for REST calls)
  5. Delete the remote branch from GitHub (Optionally)
  6. Remove the Preview Deployment from Vercel
  7. Remove the Neon branch

Maybe slightly less of a pain to do locally than option 1. GH Actions could also help here to wire all that.

cwognum commented 9 months ago

I thought about the two (three, if you count the mock hub API) options you described as well, but I ultimately landed on what I described above because I question whether we need to completely setup and teardown a new deployment every-time we run a suite of tests. I suspect that having a deployment per release will suffice. If that's not frequent enough and the DB builds up too much, we could use the Github CICD to regularly reset the test deployment, e.g. every night.

Having a dedicated test environment would also allow external collaborators without access to the hub code base to run the testing suite, right?

jstlaurent commented 9 months ago

I question whether we need to completely setup and teardown a new deployment every-time we run a suite of tests.

Well, Vercel creates a new deployment for every commit that's pushed to any branch, and their infrastructure is designed to enable that. The capability is there, I would leverage it. Besides, that's actually fairly fundamental to Vercel's model. We don't "maintain" deployments, not even prod. There's a new one every time we make a change.

Beside, setting up a new environment is typically what we do for integration testing, only we do it locally by spinning up a containerized DB. In this case, having a DB branch from prod adds a level of randomness, but it's probably worth it to get the most realistic environment.

I mean, we could do that too. If it's easier to containerize the Hub application and let you run it in GitHub Actions as a service when you launch a workflow (or locally to run in Docker), we can probably make it happen.

I suspect that having a deployment per release will suffice. If that's not frequent enough and the DB builds up too much, we could use the Github CICD to regularly reset the test deployment, e.g. every night.

Per Polaris client release, you mean?

To me, "resetting the test deployment" would mean destroying the existing one and creating a new one.

Having a dedicated test environment would also allow external collaborators without access to the hub code base to run the testing suite, right?

You mean, if we open-up the REST API to third-parties? There's many ways to go about enabling third-party testing, when we get there, with various degree of load on the team. An OpenAPI spec they can use to auto-generate a mock Hub would be my first step. Client-facing sandboxed environments with semi-ephemeral data would be... many steps further, lets say.

cwognum commented 9 months ago

Thank you for the elaborate answers, @jstlaurent! I think we're saying almost the same thing. I'll ping you on Slack for some final questions. I think it will be easier to discuss in person! 🙂