ci: add integration test to lint_test_build #2569
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -102,6 +102,27 @@ jobs: | |
| run: | | ||
| npx @stoplight/spectral-cli lint ./packages/token-introspection/openapi/*.yaml | ||
|
|
||
| integration-test: | ||
| runs-on: ubuntu-22.04 | ||
| needs: checkout | ||
| timeout-minutes: 5 | ||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Setup environment | ||
| uses: ./.github/workflows/rafiki/env-setup | ||
|
|
||
| - name: Setup hosts | ||
| run: | | ||
| echo "127.0.0.1 host.docker.internal" | sudo tee -a /etc/hosts | ||
|
Comment on lines
+116
to
+118
There was a problem hiding this comment. We're eventually going to use hostile to do this in the There was a problem hiding this comment. I'm not sure I'm following. Why do we need it twice? Because that way, the script itself doesn't prompt? There was a problem hiding this comment. 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? There was a problem hiding this comment. 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 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:
So I'm not really seeing a value-add over checking if a line already exists and if not adding it. There was a problem hiding this comment. I guess I forgot where I was... editing I will test using removing this CI step in that branch when this is merged. |
||
|
|
||
| - name: Build dependencies | ||
| run: pnpm --filter integration build:deps | ||
|
|
||
| - name: Run tests | ||
| run: pnpm --filter integration run-tests | ||
|
|
||
| build: | ||
| runs-on: ubuntu-22.04 | ||
| timeout-minutes: 5 | ||
|
|
@@ -120,5 +141,6 @@ jobs: | |
| runs-on: ubuntu-22.04 | ||
| needs: | ||
| - build | ||
| - integration-test | ||
| steps: | ||
| - run: echo 'PR Checks Passed' | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.