ci: add integration test to lint_test_build

[BlairCurrey]

Changes proposed in this pull request

• adds integration tests to ci

Context

fixes #2473

Checklist

• Related issues linked using fixes #number
• Tests added/updated
• Documentation added
• Make sure that all checks pass
• Bruno collection updated

[netlify]

✅ Deploy Preview for brilliant-pasca-3e80ec canceled.

Name	Link
🔨 Latest commit	c34b79d
🔍 Latest deploy log	https://app.netlify.com/sites/brilliant-pasca-3e80ec/deploys/65fd8dd43bab6000084eaabd

[BlairCurrey]

from docker-compose -> docker compose

This is the same for me locally, but in github actions docker-compose is v1 and finds our compose files invalid (because of the top-level name attribute). docker compose is v2.

[BlairCurrey]

Originally this was a separate workflow file but I figured I'd include it here since it's the same event trigger.

I considered making this depend on auth, backend, token-introspection but decided not to because I think it gives us a useful signal if integration tests pass and auth/backend do not. This could happen if the failures just dont overlap with the tests cases, or if the backend/auth tests are flakey. Additionally, not making it depend on our longest running jobs ensures that we aren't increasing the time it takes to run CI. The integration tests take ~2 minutes, so they should finish before backend.

[BlairCurrey]

We're eventually going to use hostile to do this in the run-tests script but I think this will end up being permanent for the reason outlined in the end of the body in this issue: #2555

[sabineschaller]

I'm not sure I'm following. Why do we need it twice? Because that way, the script itself doesn't prompt?

[mkurapov]

Same question here. In the CI, if "echo "127.0.0.1 host.docker.internal" | sudo tee -a /etc/hosts" command works without prompting, why would it prompt if it was inside the script?

[BlairCurrey]

Good questions - short answer is that I wonder if hostile will work fine in CI after all, in which case we can get rid of this step. I will test that.

The thinking was hostile will not work in github actions due to sudo prompt (needs confirming). Thus the extra step. Dont ask me about sudo tee I guess I just thought that was special 🙃. And that appending to /etc/hosts and otherwise manually performing a "append if not exist" was better managed by something like hostile, thus not using echo "127.0.0.1 host.docker.internal" | sudo tee -a /etc/hosts. And to answer Max's question, that does actually prompt from the command line - that's what makes me think my original assumptions about github actions and sudo might just be bad.

Also questioning if need hostile at this point. We are already checking for the existence of some line before setting a new one, so we shouldn't be adding duplicates. Beyond that and being cross-platform, which I don't think matters because we're already assuming a unix-like environment (including WSL) with this shell script and elswhere, it basically just does this:

• erroring if you try to overwrite local or localhost or the cli args are missing https://github.com/feross/hostile/blob/master/bin/cmd.js#L62-L70
• not setting if it exists (we're already checking that) https://github.com/feross/hostile/blob/master/index.js#L93
• adding a newline such that it preserves any existing whitespace at the end of the file. https://github.com/feross/hostile/blob/master/index.js#L94-L101

So I'm not really seeing a value-add over checking if a line already exists and if not adding it.

[BlairCurrey]

I guess I forgot where I was... editing /etc/hosts via hostile in run-tests is not in this branch and adding host step is required regardless here.

I will test using removing this CI step in that branch when this is merged.

[BlairCurrey]

Presumably backend (and auth) gql APIs will soon require a hydra service which is not included in these tests. So the integration tests will fail at that point and we will need to add hydra to the docker environment and handle authentication in the tests.

We should decide which we want to merge first. Either we merge this first and the authentication changes will be blocked until the testenv and tests handle it, or merge the authentication changes and this will be blocked. It seems like we can probably piggy-back off either of these tasks to authenticate with the tests: #2568 #2557

I think being a much larger change, maybe we merge the authentication first (one less thing to implement there). But I don't know how close it is.

[BlairCurrey]

Presumably backend (and auth) gql APIs will soon require a hydra service which is not included in these tests. So the integration tests will fail at that point and we will need to add hydra to the docker environment and handle authentication in the tests.

We should decide which we want to merge first. Either we merge this first and the authentication changes will be blocked until the testenv and tests handle it, or merge the authentication changes and this will be blocked. It seems like we can probably piggy-back off either of these tasks to authenticate with the tests: #2568 #2557

I think being a much larger change, maybe we merge the authentication first (one less thing to implement there). But I don't know how close it is.

We talked about this in sprint planning and concluded the tests should be runnable w/out spinning up hydra. should work with a NODE_ENV that is test/development and using a special header. So the tests will break with those changes but we can simply update the header in the test requests. Given the changes required are small and authetnication changes dont look imminent I dont think we need to hold of on merging this IMO.

edit: more specifically it looks like we'll just need to add this to the ApolloClient options in test/integration/lib/apolloClient.ts:

    headers: {
      authorization: 'Bearer test_file'
    }

[sabineschaller]

Just one clarifying question but otherwise, LGTM

[sabineschaller]

I'm not sure I'm following. Why do we need it twice? Because that way, the script itself doesn't prompt?

[mkurapov]

Approving for now, just one comment

[mkurapov]

Same question here. In the CI, if "echo "127.0.0.1 host.docker.internal" | sudo tee -a /etc/hosts" command works without prompting, why would it prompt if it was inside the script?