Closed thomashoneyman closed 1 year ago
Well, the integration test looks great, but the normal tests suddenly have a timeout running in GitHub Actions. Taking a look at that (they run fine on my machine, including the same command — nix develop --command run-tests-script
— and I'm not sure what about this PR would change the test — it doesn't change the mock effects).
Note: if we ever get Pursuit integrated into this repository then we can get rid of wiremock for pursuit and hit the actual pursuit machine instead.
This PR implements the integration test for the publish pipeline. In the future we should add one integration test per major registry operation (publish, transfer, unpublish, etc.). It also adds test documentation to the CONTRIBUTING.md file.
In Action
See the logs from the full integration test in this PR here: https://garnix.io/build/50RwJlPB
You can see we exercise everything in the publish workflow, from cloning, solving, compiling, uploading to storage, uploading to Pursuit, and even mirroring to the legacy package sets repository.
Mock Effects vs. Integration Tests
I think it's worth briefly describing the difference between the "integration tests" (ie. the pipeline tests written with mock effects, as seen in the app workspace's test suite) and the integration test written here.
The "integration tests" we already have in place use mock effects, in which we provide alternate handlers to
Run
for effects likeSTORAGE
orLOG
so that they don't manipulate real-world resources. These alternate handlers might skip operations altogether (likeLOG
), or they might return fixture data (likeSTORAGE
). But they still let us test the actual implementations of the various functions in the app such aspublish
.These tests run via
spago test
, using Node on your local machine. They're fast and it's easy to devise complicated scenarios to test — such as publishing, unpublishing, and then trying to republish. We should use these "integration tests" as much as possible instead of the main integration test.The integration test implemented here, however, is real-world. It spins up a virtual machine running the same NixOS configuration that we deploy via Colmena, which includes the production build of the registry-server service. We're using the production handlers for
Run
and trying to actually manipulate the various remote resources the registry connects to. Instead of testing any particular function, we're testing the API that the registry-server exposes by e.g. sending a POST request to the/publish
endpoint.This is much more difficult, slow, and error-prone to work on than the "integration tests", but it's as close as we can possibly get to the real environment the server runs in. The integration test caught me several times, for example, forgetting to ensure a particular file is in place on the server machine (like the DHALL_TYPES). We should use this test to ensure the major components of the registry work, but leave the more complex testing to the mock effects.
Mocking External Services
The registry connects to several remote resources, including the GitHub API, Pursuit API, packages.registry.purescript.org, and the underlying S3 bucket API. It also uses the
git
CLI directly to clone packages from remote Git repos. In the integration test we don't want to (and can't!) directly access any of these services. Instead, we leverage two things:gitMock
, a wrapper I wrote forgit
that replaces CLI arguments that point to remote git servers over https:// with local file:// locations, — so that commands likegit clone https://github.com/purescript/registry.git
are rewritten togit clone file:///fixtures/purescript/registry
instead — but otherwise proxies everything to the normalgit
binary. When you build the registry-server derivation for the integration test VM we replacegit
withgitMock
(but only for that derivation).wiremock
, a useful package for mocking HTTP APIs. You can give it a list of request/response pairs and it will serve them over localhost. All you need to do is change the base URL of the API you want to hit to point to the wiremock service instead. In our case, we have four instances of wiremock running — one per remote API we hit.The only significant change this introduces to the PureScript codebase is that the server no longer hardcodes API locations. Instead, the base URLs for the four APIs we use are all configurable via env variables (such as
PURSUIT_API_URL
). Since we have good defaults for these env vars when not in a test environment, I've hardcoded those defaults into theEnv
module.Future Work
For now I have only implemented the publish operation. Once we feel comfortable merging this test, though, I'd like to expand to include the transfer, unpublish, and package set update operations. At that point I don't feel we should add anything new to the integration test; it doesn't need to be huge, it just needs to make sure the core operations of the registry work as expected.