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.

Sign up for GitHub

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

Jump to bottom

Skip fixture tests #6770

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Skip fixture tests #6770

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
1196a22
test
penalosa Sep 19, 2024
433c783
test
penalosa Sep 19, 2024
231b793
test
penalosa Sep 19, 2024
c8e9cbb
only run fixtures with the fixtures label
penalosa Sep 19, 2024
ac7e344
limit tests when only md file have been changed
penalosa Sep 19, 2024
ca30779
Only run tools tests on linux
penalosa Sep 19, 2024
fab3a85
auto apply when fixtures changed
penalosa Sep 19, 2024
a9002d6
skips steps not the job
penalosa Sep 19, 2024
d3a5646
Revert "skips steps not the job"
penalosa Sep 19, 2024
f94f31f
timeout -> 30
penalosa Sep 19, 2024
9e52378
Only validate PR description if non-md files are changed
penalosa Sep 19, 2024
a753495
Merge branch 'main' into penalosa/skip-fixtures
penalosa Sep 19, 2024
240db17
move changeset test to tools
penalosa Sep 19, 2024
83e5c22
types
penalosa Sep 19, 2024
2f5025b
run fixtures
penalosa Sep 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .github/pr-labeler.config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,8 @@ e2e:
skip-pr-description-validation:
- all:
- head-branch: "changeset-release/main"

fixtures:
- all:
- changed-files:
- any-glob-to-any-file: "fixtures/**/*"
2 changes: 1 addition & 1 deletion .github/workflows/e2e.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
group: ${{ github.workflow }}-${{ github.ref }}-${{ matrix.os }}-${{ matrix.node }}
cancel-in-progress: true
timeout-minutes: 40
if: github.repository_owner == 'cloudflare' && (github.event_name != 'pull_request' || (github.event_name == 'pull_request' && contains(github.event.*.labels.*.name, 'e2e' )))
if: (github.event_name != 'pull_request' || (github.event_name == 'pull_request' && contains(github.event.*.labels.*.name, 'e2e' ) && github.event.pull_request.head.repo.owner.login == 'cloudflare'))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Turns out repository_owner is always cloudflare, even for forks

name: "E2E Test"
strategy:
fail-fast: false
Expand Down
78 changes: 71 additions & 7 deletions .github/workflows/test-and-check.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
name: Tests + Checks

on: pull_request
on:
pull_request:
types: [opened, synchronize, reopened, labeled, unlabeled]

jobs:
add-to-project:
if: github.ref != 'refs/heads/changeset-release/main'
if: github.head_ref != 'changeset-release/main'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Turns out github.ref for pull_request events refers to the pull request ref (e.g. refs/6770/merge)

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}-add-pr
cancel-in-progress: true
Expand All @@ -15,7 +17,7 @@ jobs:
- run: curl -X POST https://devprod-status-bot.devprod.workers.dev/pr-project/workers-sdk/${{ github.event.number }}

check-non-linux:
if: github.ref == 'refs/heads/changeset-release/main'
if: github.head_ref == 'changeset-release/main'
timeout-minutes: 30
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}-${{ matrix.os }}-checks
Expand Down Expand Up @@ -89,7 +91,7 @@ jobs:
- name: Check the changesets
run: node -r esbuild-register tools/deployments/validate-changesets.ts

test:
fixtures:
timeout-minutes: 30
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}-${{ matrix.os }}-${{ matrix.filter }}-test
Expand All @@ -100,8 +102,9 @@ jobs:
fail-fast: false
matrix:
os: [ubuntu-latest, windows-latest, macos-13]
filter: ["./fixtures/*", "./packages/*", "./tools"]
filter: ["./fixtures/*"]
runs-on: ${{ matrix.os }}
if: github.head_ref == 'changeset-release/main' || contains(github.event.*.labels.*.name, 'fixtures' )
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the pertinent change—only run fixtures if the fixtures label is applied

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we auto-apply this label under certain circumstances?

eg. when the fixtures dir has been touched, when wrangler/src has been touched, etc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fab3a85

steps:
- name: Checkout Repo
uses: actions/checkout@v4
Expand All @@ -126,8 +129,69 @@ jobs:
TEST_REPORT_PATH: ${{ runner.temp }}/test-report/index.html
CI_OS: ${{ runner.os }}

- name: Run changeset version tests
run: node .github/changeset-version.test.js
- name: Upload debug logs
if: always()
uses: actions/upload-artifact@v3
with:
name: e2e-test-debug-logs-${{ matrix.os }}-${{ matrix.node }}
path: ${{ runner.temp }}/wrangler-debug-logs/

- name: Upload test report
if: always()
uses: actions/upload-artifact@v3
with:
name: e2e-test-report-${{ matrix.os }}-${{ matrix.node }}
path: ${{ runner.temp }}/test-report/

test:
timeout-minutes: 30
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}-${{ matrix.os }}-${{ matrix.filter }}-test
cancel-in-progress: true

name: "Tests"
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, windows-latest, macos-13]
filter: ["./packages/*"]
# Things in the tools folder are for running in CI, and so only need to run on linux
include:
- os: ubuntu-latest
filter: "./tools"
edmundhung marked this conversation as resolved.
Show resolved Hide resolved
runs-on: ${{ matrix.os }}
steps:
- name: Checkout Repo
uses: actions/checkout@v4
with:
fetch-depth: 0

- uses: dorny/paths-filter@v3
id: changes
with:
filters: |
everything_but_markdown:
- '!**/*.md'

Comment on lines +117 to +123
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only run the remaining tests if a non-markdown file was changed, allowing e.g. README fixes to be light on CI time

- name: Install Dependencies
if: steps.changes.outputs.everything_but_markdown == 'true'
uses: ./.github/actions/install-dependencies
with:
turbo-api: ${{ secrets.TURBO_API }}
turbo-team: ${{ secrets.TURBO_TEAM }}
turbo-token: ${{ secrets.TURBO_TOKEN }}
turbo-signature: ${{ secrets.TURBO_REMOTE_CACHE_SIGNATURE_KEY }}

- name: Run tests
if: steps.changes.outputs.everything_but_markdown == 'true'
run: pnpm run test:ci --concurrency 1 --filter=${{ matrix.filter }}
env:
TMP_CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }}
TMP_CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }}
NODE_OPTIONS: "--max_old_space_size=8192"
WRANGLER_LOG_PATH: ${{ runner.temp }}/wrangler-debug-logs/
TEST_REPORT_PATH: ${{ runner.temp }}/test-report/index.html
CI_OS: ${{ runner.os }}

- name: Upload debug logs
if: always()
Expand Down
11 changes: 10 additions & 1 deletion .github/workflows/validate-pr-description.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ on:

jobs:
check:
if: github.ref != 'refs/heads/changeset-release/main'
if: github.head_ref != 'changeset-release/main'
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}-add-pr
cancel-in-progress: true
Expand All @@ -19,7 +19,15 @@ jobs:
with:
fetch-depth: 0

- uses: dorny/paths-filter@v3
id: changes
with:
filters: |
everything_but_markdown:
- '!**/*.md'

- name: Install Dependencies
if: steps.changes.outputs.everything_but_markdown == 'true'
uses: ./.github/actions/install-dependencies
with:
turbo-api: ${{ secrets.TURBO_API }}
Expand All @@ -28,6 +36,7 @@ jobs:
turbo-signature: ${{ secrets.TURBO_REMOTE_CACHE_SIGNATURE_KEY }}

- run: node -r esbuild-register tools/deployments/validate-pr-description.ts
if: steps.changes.outputs.everything_but_markdown == 'true'
env:
TITLE: ${{ github.event.pull_request.title }}
BODY: ${{ github.event.pull_request.body }}
Expand Down
34 changes: 15 additions & 19 deletions .github/changeset-version.test.js → ...ments/__tests__/changeset-version.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const assert = require("node:assert");
const { getNextMiniflareVersion } = require("./changeset-version.js");
import { expect, it } from "vitest";
// @ts-expect-error This is a JS file
import { getNextMiniflareVersion } from "../../../.github/changeset-version";

Comment on lines +2 to +3
Copy link
Contributor

Choose a reason for hiding this comment

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

you should be able to import js from ts

// prettier-ignore
const miniflareVersionTestCases = [
Expand All @@ -14,24 +15,19 @@ const miniflareVersionTestCases = [
["1.20231008.0", "3.20231001.0", /* major */ "4.0.0", "4.20231008.0"],
];

for (const [
workerdVersion,
previousMiniflareVersion,
miniflareVersion,
correctMiniflareVersion,
] of miniflareVersionTestCases) {
const actual = getNextMiniflareVersion(
it.each(miniflareVersionTestCases)(
"changeset version",
(
workerdVersion,
previousMiniflareVersion,
miniflareVersion
);
assert.strictEqual(
actual,
correctMiniflareVersion,
`Expected "${correctMiniflareVersion}" with ${JSON.stringify({
miniflareVersion,
correctMiniflareVersion
) => {
const actual = getNextMiniflareVersion(
workerdVersion,
previousMiniflareVersion,
miniflareVersion,
})}, got "${actual}"`
);
}
miniflareVersion
);
expect(actual).toEqual(correctMiniflareVersion);
}
);
Loading