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

Use Turborepo for tests and add turbo cache for CI eslint and unit-tests #3458

Merged
merged 5 commits into from
Apr 12, 2022

Conversation

menghif
Copy link
Contributor

@menghif menghif commented Apr 11, 2022

Issue This PR Addresses

Fixes #3351

Type of Change

  • Bugfix: Change which fixes an issue
  • New Feature: Change which adds functionality
  • Documentation Update: Change which improves documentation
  • UI: Change which improves UI

Description

This PR changes pnpm test to use Turborepo and adds support for Turborepo's cache in CI eslint and unit-tests.

The downside of this is not seeing all the green check marks when running tests locally, so I have left the option to run tests without Turborepo by running pnpm jest.

For CI I am using a GitHub Action that builds a local server (on port 9080) where Turborepo goes to look for the cache. To make it work, a TURBO_SERVER_TOKEN needs to be added to the repo.

If this is successful, we will be able to do the same for our e2e tests.

Resources:
Custom Remote Cache
TurboRepo with github artifacts

Steps to test the PR

pnpm test

Checklist

  • Quality: This PR builds and passes our npm test and works locally
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not (if applicable)
  • Documentation: This PR includes updated/added documentation to user exposed functionality or configuration variables are added/changed or an explanation of why it does not(if applicable)

@gitpod-io
Copy link

gitpod-io bot commented Apr 11, 2022

@menghif menghif self-assigned this Apr 11, 2022
Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

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

This is awesome.

Should we do a follow-up to run a turbo-repo cache server on staging for local devs to leverage too? Or is this not needed locally?

uses: felixmosh/turborepo-gh-artifacts@v1
with:
repo-token: ${{ secrets.GITHUB_TOKEN }}
server-token: ${{ secrets.TURBO_SERVER_TOKEN }}
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this token supposed to look like? I'm reading the docs and I don't totally understand the point of it.

Copy link
Contributor

Choose a reason for hiding this comment

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

The answer is: it's an opaque secret token (can be anything), see felixmosh/turborepo-gh-artifacts#1 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

I've added TURBO_SERVER_TOKEN to the repo.

@menghif
Copy link
Contributor Author

menghif commented Apr 11, 2022

This is awesome.

Should we do a follow-up to run a turbo-repo cache server on staging for local devs to leverage too? Or is this not needed locally?

I don't think it's needed for local dev since Turborepo keeps a local cache in a .turbo folder.

Also there is a new feature in version 1.2 to add signature verification:
https://turborepo.org/blog/turbo-1-2-0#cache-outputs-integrity-and-signature-verification
Is this something we should add?

- name: Install dependencies and run eslint
run: |
pnpm install
pnpm lint
pnpm turbo run lint --api="http://127.0.0.1:9080" --token="${{ secrets.TURBO_SERVER_TOKEN }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

You can't access secrets.* directly in a called workflow. You have to pass the secret into the workflow. Checkout how I do it with the Docker username and password:

Do this for both of the workflow changes you're making.

@humphd
Copy link
Contributor

humphd commented Apr 11, 2022

This is awesome.
Should we do a follow-up to run a turbo-repo cache server on staging for local devs to leverage too? Or is this not needed locally?

I don't think it's needed for local dev since Turborepo keeps a local cache in a .turbo folder.

Also there is a new feature in version 1.2 to add signature verification: https://turborepo.org/blog/turbo-1-2-0#cache-outputs-integrity-and-signature-verification Is this something we should add?

We're not uploading them to a cache, right? I'm not sure we need this.

@menghif
Copy link
Contributor Author

menghif commented Apr 11, 2022

This is awesome.
Should we do a follow-up to run a turbo-repo cache server on staging for local devs to leverage too? Or is this not needed locally?

I don't think it's needed for local dev since Turborepo keeps a local cache in a .turbo folder.
Also there is a new feature in version 1.2 to add signature verification: https://turborepo.org/blog/turbo-1-2-0#cache-outputs-integrity-and-signature-verification Is this something we should add?

We're not uploading them to a cache, right? I'm not sure we need this.

Right, it would only be useful if we used an actual server.

.github/workflows/eslint-ci.yml Outdated Show resolved Hide resolved
@@ -2,6 +2,13 @@ name: Unit Tests Workflow

on:
workflow_call:
secrets:
github-token:
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, bump these in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just did. I don't understand why CI fails to start, I followed your docker example.

Copy link
Contributor

Choose a reason for hiding this comment

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

Invalid workflow file: .github/workflows/continuous-integration.yml#L22
The workflow is not valid. .github/workflows/continuous-integration.yml (Line: 22, Col: 21): Invalid secret, github-token is not defined in the referenced workflow. .github/workflows/continuous-integration.yml (Line: 23, Col: 24): Invalid secret, turborepo-token is not defined in the referenced workflow.

From https://github.com/Seneca-CDOT/telescope/actions/runs/2150363645

Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

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

This looks right to me. NOTE: we don't run linked workflows from PRs, only what's on master, so we have to land this to see it happen.

Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

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

I think it's uphappy with the GITHUB_TOKEN stuff

@@ -18,10 +18,16 @@ jobs:
# Make sure eslint passes
eslint:
uses: Seneca-CDOT/telescope/.github/workflows/eslint-ci.yml@master
secrets:
github-token: ${{ secrets.GITHUB_TOKEN }}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can't pass this in (it's a synthesized token, not a real one), see https://github.community/t/how-to-pass-github-token-to-reusable-workflows/204905/6. What if you use {{ github.token }} below instead and get rid of this.

DukeManh
DukeManh previously approved these changes Apr 12, 2022
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.

Use Turborepo for tests
4 participants