Skip to content

Commit

Permalink
Speed up github actions for PRs (#7250)
Browse files Browse the repository at this point in the history
Co-authored-by: Michaël De Boey <[email protected]>
  • Loading branch information
brophdawg11 and MichaelDeBoey committed Aug 25, 2023
1 parent 85ce069 commit a563b97
Show file tree
Hide file tree
Showing 11 changed files with 315 additions and 32 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/reusable-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ jobs:
fail-fast: false
matrix:
node: ${{ fromJSON(inputs.node_version) }}
browser: ["edge"]
browser: ["msedge"]

runs-on: windows-latest
steps:
Expand Down
36 changes: 36 additions & 0 deletions .github/workflows/shared-build.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
name: 🛠️ Build

on:
workflow_call:

env:
CI: true
CYPRESS_INSTALL_BINARY: 0

jobs:
build:
runs-on: ubuntu-latest
steps:
- name: 🛑 Cancel Previous Runs
uses: styfle/[email protected]

- name: ⬇️ Checkout repo
uses: actions/checkout@v3

- name: ⎔ Setup node
uses: actions/setup-node@v3
with:
node-version-file: ".nvmrc"
cache: "yarn"

- name: Disable GitHub Actions Annotations
run: |
echo "::remove-matcher owner=tsc::"
echo "::remove-matcher owner=eslint-compact::"
echo "::remove-matcher owner=eslint-stylish::"
- name: 📥 Install deps
run: yarn --frozen-lockfile

- name: 🏗 Build
run: yarn build
62 changes: 62 additions & 0 deletions .github/workflows/shared-test-integration.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
name: 🧪 Test (Integration)

on:
workflow_call:
inputs:
os:
required: true
type: string
node_version:
required: true
# this is limited to string | boolean | number (https://github.community/t/can-action-inputs-be-arrays/16457)
# but we want to pass an array (node_version: "[18, 20]"),
# so we'll need to manually stringify it for now
type: string
browser:
required: true
# this is limited to string | boolean | number (https://github.community/t/can-action-inputs-be-arrays/16457)
# but we want to pass an array (browser: "['chromium', 'firefox']"),
# so we'll need to manually stringify it for now
type: string

env:
CI: true
CYPRESS_INSTALL_BINARY: 0

jobs:
integration:
name: "${{ inputs.os }} / node@${{ matrix.node }} / ${{ matrix.browser }}"
strategy:
fail-fast: false
matrix:
node: ${{ fromJSON(inputs.node_version) }}
browser: ${{ fromJSON(inputs.browser) }}

runs-on: ${{ inputs.os }}
steps:
- name: 🛑 Cancel Previous Runs
uses: styfle/[email protected]

- name: ⬇️ Checkout repo
uses: actions/checkout@v3

- name: ⎔ Setup node ${{ matrix.node }}
uses: actions/setup-node@v3
with:
node-version: ${{ matrix.node }}
cache: "yarn"

- name: Disable GitHub Actions Annotations
run: |
echo "::remove-matcher owner=tsc::"
echo "::remove-matcher owner=eslint-compact::"
echo "::remove-matcher owner=eslint-stylish::"
- name: 📥 Install deps
run: yarn --frozen-lockfile

- name: 📥 Install Playwright
run: npx playwright install --with-deps ${{ matrix.browser }}

- name: 👀 Run Integration Tests ${{ matrix.browser }}
run: "yarn test:integration --project=${{ matrix.browser }}"
55 changes: 55 additions & 0 deletions .github/workflows/shared-test-unit.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
name: 🧪 Test (Unit)

on:
workflow_call:
inputs:
os:
required: true
type: string
node_version:
required: true
# this is limited to string | boolean | number (https://github.community/t/can-action-inputs-be-arrays/16457)
# but we want to pass an array (node_version: "[18, 20]"),
# so we'll need to manually stringify it for now
type: string

env:
CI: true
CYPRESS_INSTALL_BINARY: 0

jobs:
test:
name: "${{ inputs.os }} / node@${{ matrix.node }}"
strategy:
fail-fast: false
matrix:
node: ${{ fromJSON(inputs.node_version) }}
runs-on: ${{ inputs.os }}
steps:
- name: 🛑 Cancel Previous Runs
uses: styfle/[email protected]

- name: ⬇️ Checkout repo
uses: actions/checkout@v3

- name: ⎔ Setup node ${{ matrix.node }}
uses: actions/setup-node@v3
with:
node-version: ${{ matrix.node }}
cache: "yarn"

- name: Disable GitHub Actions Annotations
run: |
echo "::remove-matcher owner=tsc::"
echo "::remove-matcher owner=eslint-compact::"
echo "::remove-matcher owner=eslint-stylish::"
- name: 📥 Install deps
run: yarn --frozen-lockfile

# It's faster to use the built `cli.js` in tests if its available and up-to-date
- name: 🏗 Build
run: yarn build

- name: 🧪 Run Primary Tests
run: "yarn test:primary"
63 changes: 63 additions & 0 deletions .github/workflows/test-full.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
name: Branch

# main/dev branches will get the full run across all OS/browsers

on:
push:
branches:
- main
- dev
paths-ignore:
- "docs/**"
- "scripts/**"
- "contributors.yml"
- "**/*.md"

jobs:
build:
name: "⚙️ Build"
if: github.repository == 'remix-run/remix'
uses: ./.github/workflows/shared-build.yml

unit-ubuntu:
name: "🧪 Unit Test"
if: github.repository == 'remix-run/remix'
uses: ./.github/workflows/shared-test-unit.yml
with:
os: "ubuntu-latest"
node_version: '["latest"]'

unit-windows:
name: "🧪 Unit Test"
if: github.repository == 'remix-run/remix'
uses: ./.github/workflows/shared-test-unit.yml
with:
os: "windows-latest"
node_version: '["latest"]'

integration-ubuntu:
name: "👀 Integration Test"
if: github.repository == 'remix-run/remix'
uses: ./.github/workflows/shared-test-integration.yml
with:
os: "ubuntu-latest"
node_version: '["latest"]'
browser: '["chromium", "firefox"]'

integration-windows:
name: "👀 Integration Test"
if: github.repository == 'remix-run/remix'
uses: ./.github/workflows/shared-test-integration.yml
with:
os: "windows-latest"
node_version: '["latest"]'
browser: '["msedge"]'

integration-macos:
name: "👀 Integration Test"
if: github.repository == 'remix-run/remix'
uses: ./.github/workflows/shared-test-integration.yml
with:
os: "macos-latest"
node_version: '["latest"]'
browser: '["webkit"]'
34 changes: 34 additions & 0 deletions .github/workflows/test-pr-ubuntu.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
name: PR (Base)

# All PRs touching code will run tests on ubuntu/node/chromium

on:
pull_request:
paths-ignore:
- "docs/**"
- "scripts/**"
- "contributors.yml"
- "**/*.md"

jobs:
build:
name: "⚙️ Build"
if: github.repository == 'remix-run/remix'
uses: ./.github/workflows/shared-build.yml

unit-ubuntu:
name: "🧪 Unit Test"
if: github.repository == 'remix-run/remix'
uses: ./.github/workflows/shared-test-unit.yml
with:
os: "ubuntu-latest"
node_version: '["latest"]'

integration-chromium:
name: "👀 Integration Test"
if: github.repository == 'remix-run/remix'
uses: ./.github/workflows/shared-test-integration.yml
with:
os: "ubuntu-latest"
node_version: '["latest"]'
browser: '["chromium"]'
46 changes: 46 additions & 0 deletions .github/workflows/test-pr-windows-macos.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
name: PR (Full)

# PRs touching create-remix/remix-dev will also run on Windows and OSX

on:
pull_request:
paths:
- "packages/create-remix/**"
- "packages/remix-dev/**"
- "!**/*.md"

jobs:
unit-windows:
name: "🧪 Unit Test"
if: github.repository == 'remix-run/remix'
uses: ./.github/workflows/shared-test-unit.yml
with:
os: "windows-latest"
node_version: '["latest"]'

integration-firefox:
name: "👀 Integration Test"
if: github.repository == 'remix-run/remix'
uses: ./.github/workflows/shared-test-integration.yml
with:
os: "ubuntu-latest"
node_version: '["latest"]'
browser: '["firefox"]'

integration-msedge:
name: "👀 Integration Test"
if: github.repository == 'remix-run/remix'
uses: ./.github/workflows/shared-test-integration.yml
with:
os: "windows-latest"
node_version: '["latest"]'
browser: '["msedge"]'

integration-webkit:
name: "👀 Integration Test"
if: github.repository == 'remix-run/remix'
uses: ./.github/workflows/shared-test-integration.yml
with:
os: "macos-latest"
node_version: '["latest"]'
browser: '["webkit"]'
25 changes: 0 additions & 25 deletions .github/workflows/test.yml

This file was deleted.

13 changes: 10 additions & 3 deletions integration/fetcher-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,11 @@ test.describe("useFetcher", () => {
method: "get",
}),
]);

await page.waitForSelector(`pre:has-text("${LUNCH}")`);
// Check full HTML here - Chromium/Firefox/Webkit seem to render this in
// a <pre> but Edge puts it in some weird code editor markup:
// <body data-code-mirror="Readonly code editor.">
// <div hidden="true">"LUNCH"</div>
expect(await app.getHtml()).toContain(LUNCH);
});

test("Form can hit an action", async ({ page }) => {
Expand All @@ -258,7 +261,11 @@ test.describe("useFetcher", () => {
method: "post",
}),
]);
await page.waitForSelector(`pre:has-text("${CHEESESTEAK}")`);
// Check full HTML here - Chromium/Firefox/Webkit seem to render this in
// a <pre> but Edge puts it in some weird code editor markup:
// <body data-code-mirror="Readonly code editor.">
// <div hidden="true">"LUNCH"</div>
expect(await app.getHtml()).toContain(CHEESESTEAK);
});
});

Expand Down
2 changes: 1 addition & 1 deletion integration/hmr-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,7 @@ whatsup
// let expectedErrorCount = 0;
let expectDestructureTypeError = expectConsoleError((error) => {
let expectedMessage = new Set([
// chrome, edge
// chrome, msedge
"Cannot destructure property 'hello' of 'useLoaderData(...)' as it is null.",
// firefox
"(intermediate value)() is null",
Expand Down
9 changes: 7 additions & 2 deletions integration/playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,13 @@ const config: PlaywrightTestConfig = {
use: devices["Desktop Safari"],
},
{
name: "edge",
use: devices["Desktop Edge"],
name: "msedge",
use: {
...devices["Desktop Edge"],
// Desktop Edge uses chromium by default
// https://github.com/microsoft/playwright/blob/main/packages/playwright-core/src/server/deviceDescriptorsSource.json#L1502
channel: "msedge",
},
},
{
name: "firefox",
Expand Down

0 comments on commit a563b97

Please sign in to comment.