Skip to content

Commit

Permalink
Move Browserstack into a seperate workflow (#1126)
Browse files Browse the repository at this point in the history
### Changes

Opening this PR as draft as a discussion starter to see if what I'm
doing here makes sense and would be something we would want.

The PR separates Browserstack into its own workflow, doing that removes
the need for using the `pull_request_target` trigger on the `Build and
Test` workflow, making it easier to make modifications to that workflow
file when needed.

It does keep the `pull_request_target` and `authorize` step for the
Browserstack workflow to guarantee our secrets are protected by the
means of a manual approval step.

**Why?**

Using `pull_request_target` does not run workflow changes on PRs as it
ensures it always runs in the context of the target branch. For example,
I[ opened a PR to update the NODE_VERSION to
20](#1127), but you will
notice that it still uses node 18 to execute the PR's workflow. This
means that we will only know if it works with node20 on GitHub Actions
after merging it to master.

However, the only reason we use `pull_request_target` is for
Browserstack. If we separate Browserstack into its own workflow, we no
longer need `pull_request_target` on our main `Build and Test` workflow,
meaning in that case, the PR I opened would run against node 20 on the
PR level, and not after merging.

I'm not yet sure if and how this will work for automating our release,
as I know it becomes really complicated when u have separate workflows.
But we already have the publish in a separate workflow anyway, so adding
a 3rd one doesn't necessarily add additional complexity, I think.

Unrelated, but also Drops --include=dev from npm ci, as we don't need
that. But same goes for changes like this. We have no way to identify
this works before merging this PR, because of the usage of
`pull_request_target`, which (as mentioned above) is not needed to build
our SDK and run its tests.

### Checklist

- [x] I have read the [Auth0 general contribution
guidelines](https://github.com/auth0/open-source-template/blob/master/GENERAL-CONTRIBUTING.md)
- [x] I have read the [Auth0 Code of
Conduct](https://github.com/auth0/open-source-template/blob/master/CODE-OF-CONDUCT.md)
- [x] All code quality tools/guidelines have been run/followed
  • Loading branch information
frederikprijck committed Aug 29, 2023
1 parent dc10bd1 commit e8e4f2c
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 45 deletions.
2 changes: 1 addition & 1 deletion .github/actions/build/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ runs:

- name: Install dependencies
shell: bash
run: npm ci --include=dev
run: npm ci

- name: Build package
shell: bash
Expand Down
54 changes: 54 additions & 0 deletions .github/workflows/browserstack.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
name: Browserstack

on:
merge_group:
workflow_dispatch:
pull_request_target:
types:
- opened
- synchronize
push:
branches:
- master

permissions:
contents: read

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: ${{ github.ref != 'refs/heads/master' }}

env:
NODE_VERSION: 18

jobs:
authorize:
name: Authorize
environment: ${{ github.actor != 'dependabot[bot]' && github.event_name == 'pull_request_target' && github.event.pull_request.head.repo.full_name != github.repository && 'external' || 'internal' }}
runs-on: ubuntu-latest
steps:
- run: true

browserstack:
needs: authorize

name: BrowserStack Tests
runs-on: ubuntu-latest

steps:
- name: Checkout code
uses: actions/checkout@v3
with:
ref: ${{ github.event.pull_request.head.sha || github.ref }}

- name: Build package
uses: ./.github/actions/build
with:
node: ${{ env.NODE_VERSION }}

- name: Run tests
shell: bash
run: npx concurrently --raw --kill-others --success first "npm:dev" "wait-on http://127.0.0.1:3000/ && browserstack-cypress run --build-name ${{ github.event.pull_request.head.sha || github.ref }}"
env:
BROWSERSTACK_ACCESS_KEY: ${{ secrets.BROWSERSTACK_ACCESS_KEY }}
BROWSERSTACK_USERNAME: ${{ secrets.BROWSERSTACK_USERNAME }}
48 changes: 4 additions & 44 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,9 @@ name: Build and Test
on:
merge_group:
workflow_dispatch:
pull_request_target:
types:
- opened
- synchronize
pull_request:
branches:
- master
push:
branches:
- master
Expand All @@ -31,16 +30,7 @@ env:
});
jobs:
authorize:
name: Authorize
environment: ${{ github.event_name == 'pull_request_target' && github.event.pull_request.head.repo.full_name != github.repository && 'external' || 'internal' }}
runs-on: ubuntu-latest
steps:
- run: true

test:
needs: authorize # Require approval before running on forked pull requests

name: Build Package
runs-on: ubuntu-latest

Expand Down Expand Up @@ -83,39 +73,9 @@ jobs:

- name: Run tests
run: npm run test -- --maxWorkers=2

- name: Upload coverage
uses: codecov/codecov-action@eaaf4bedf32dbdc6b720b63067d99c4d77d6047d # [email protected]

browserstack:
needs: test

name: BrowserStack Tests
runs-on: ubuntu-latest

steps:
- name: Checkout code
uses: actions/checkout@v3

- name: Setup Node
uses: actions/setup-node@v3
with:
node-version: ${{ env.NODE_VERSION }}
cache: 'npm'

- name: Restore build artifacts
uses: actions/cache/restore@v3
with:
path: .
key: ${{ env.CACHE_KEY }}

- name: Run tests
shell: bash
run: npx concurrently --raw --kill-others --success first "npm:dev" "wait-on http://127.0.0.1:3000/ && browserstack-cypress run --build-name ${{ github.ref }}-${{ github.sha }}"
env:
BROWSERSTACK_ACCESS_KEY: ${{ secrets.BROWSERSTACK_ACCESS_KEY }}
BROWSERSTACK_USERNAME: ${{ secrets.BROWSERSTACK_USERNAME }}

gatsby:
needs: test

Expand Down

0 comments on commit e8e4f2c

Please sign in to comment.