Test fixups & skip tests for docs changes #6770

penalosa · 2024-09-19T13:06:56Z

What this PR solves / how to test

Fixes #6685

Fixup tests to not run E2E tests on forks
Only run normal tests when a non-markdown file has been changed (example PR: Update README.md #6774)

Author has addressed the following

Tests
- TODO (before merge)
- Tests included
- Tests not necessary because: workflows change
E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
- I don't know
- Required
- Not required because: workflows change
Changeset (Changeset guidelines)
- TODO (before merge)
- Changeset included
- Changeset not necessary because: this shouldn't trigger a release
Public documentation
- TODO (before merge)
- Cloudflare docs PR(s):
- Documentation not necessary because: workflows change

changeset-bot · 2024-09-19T13:07:00Z

⚠️ No Changeset found

Latest commit: 9af85e3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

github-actions · 2024-09-19T13:14:32Z

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11124819674/npm-package-wrangler-6770

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/6770/npm-package-wrangler-6770

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11124819674/npm-package-wrangler-6770 dev path/to/script.js

Additional artifacts:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11124819674/npm-package-create-cloudflare-6770 --no-auto-update

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11124819674/npm-package-cloudflare-kv-asset-handler-6770

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11124819674/npm-package-miniflare-6770

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11124819674/npm-package-cloudflare-pages-shared-6770

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11124819674/npm-package-cloudflare-vitest-pool-workers-6770

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11124819674/npm-package-cloudflare-workers-editor-shared-6770

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/11124819674/npm-package-cloudflare-workers-shared-6770

Note that these links will no longer work once the GitHub Actions artifact expires.

[email protected] includes the following runtime dependencies:

Package	Constraint	Resolved
`miniflare`	workspace:*	3.20240925.0
`workerd`	1.20240925.0	1.20240925.0
`workerd --version`	1.20240925.0	2024-09-25

Please ensure constraints are pinned, and miniflare/workerd minor versions match.

penalosa · 2024-09-19T13:35:19Z

.github/workflows/e2e.yml

@@ -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'))


Turns out repository_owner is always cloudflare, even for forks

penalosa · 2024-09-19T13:35:56Z

.github/workflows/test-and-check.yml


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


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

penalosa · 2024-09-19T13:36:19Z

.github/workflows/test-and-check.yml

+        os: [ubuntu-latest, windows-latest, macos-13]
+        filter: ["./fixtures/*"]
+    runs-on: ${{ matrix.os }}
+    if: github.head_ref == 'changeset-release/main' || contains(github.event.*.labels.*.name, 'fixtures' )


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

Can we auto-apply this label under certain circumstances?

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

penalosa · 2024-09-19T13:36:59Z

.github/workflows/test-and-check.yml

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


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

This reverts commit a9002d6.

.github/workflows/test-and-check.yml

petebacondarwin · 2024-09-19T14:16:46Z

Only run fixture tests when the fixtures label is applied (or on Version Packages)

I think this is dangerous. There are a lot of very useful tests in fixtures that actually test significant functionality.
But perhaps what we need to do is pull those tests out of fixtures and put them somewhere else if we are going to not both running the tests on most PRs.

penalosa · 2024-09-19T14:24:29Z

There are a lot of very useful tests in fixtures that actually test significant functionality.

They're also really flaky, though. In most cases (especially from forks where the turbo cache isn't available) I think any signal in fixtures is lost to the noise of flakes. We'll still be running them on Version Packages, so we won't be releasing with fixture failures. Of course, this might end up in loads of things needing to be reverted/fixed in Version Packages, but if that's the case we can revisit things—it seems like it's worth a try to make opening PRs and contributing smoother?

petebacondarwin · 2024-09-19T14:45:20Z

Could we then only skip the flaky ones if this label is not applied?

penalosa · 2024-09-19T14:46:28Z

Could we then only skip the flaky ones if this label is not applied?

And keep the rest running? That could work

RamIdeas · 2024-09-27T20:19:13Z

tools/deployments/__tests__/changeset-version.test.ts

+// @ts-expect-error This is a JS file
+import { getNextMiniflareVersion } from "../../../.github/changeset-version";


you should be able to import js from ts

test

1196a22

penalosa added 2 commits September 19, 2024 14:08

test

433c783

test

231b793

penalosa added 2 commits September 19, 2024 14:28

only run fixtures with the fixtures label

c8e9cbb

limit tests when only md file have been changed

ac7e344

penalosa commented Sep 19, 2024

View reviewed changes

penalosa mentioned this pull request Sep 19, 2024

Add no-op test + check workflow #6682

Closed

12 tasks

penalosa marked this pull request as ready for review September 19, 2024 13:40

penalosa requested a review from a team as a code owner September 19, 2024 13:40

penalosa added 2 commits September 19, 2024 14:43

Only run tools tests on linux

ca30779

auto apply when fixtures changed

fab3a85

penalosa requested a review from RamIdeas September 19, 2024 13:55

penalosa added 2 commits September 19, 2024 14:57

skips steps not the job

a9002d6

Revert "skips steps not the job"

d3a5646

This reverts commit a9002d6.

edmundhung reviewed Sep 19, 2024

View reviewed changes

.github/workflows/test-and-check.yml Outdated Show resolved Hide resolved

.github/workflows/test-and-check.yml Outdated Show resolved Hide resolved

RamIdeas approved these changes Sep 19, 2024

View reviewed changes

penalosa added 3 commits September 19, 2024 15:07

timeout -> 30

f94f31f

Only validate PR description if non-md files are changed

9e52378

Merge branch 'main' into penalosa/skip-fixtures

a753495

move changeset test to tools

240db17

types

83e5c22

edmundhung approved these changes Sep 19, 2024

View reviewed changes

run fixtures

2f5025b

RamIdeas reviewed Sep 27, 2024

View reviewed changes

Update test-and-check.yml

9af85e3

penalosa changed the title ~~Skip fixture tests~~ Test fixups & skip tests for docs changes Oct 1, 2024

penalosa merged commit 9e977b5 into main Oct 2, 2024
20 of 22 checks passed

penalosa deleted the penalosa/skip-fixtures branch October 2, 2024 10:30

penalosa mentioned this pull request Oct 8, 2024

Skip C3 tests on *.md only changes #6925

Merged

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Test fixups & skip tests for docs changes #6770

Test fixups & skip tests for docs changes #6770

penalosa commented Sep 19, 2024 •

edited

Loading

changeset-bot bot commented Sep 19, 2024 •

edited

Loading

github-actions bot commented Sep 19, 2024 •

edited

Loading

penalosa Sep 19, 2024

penalosa Sep 19, 2024

penalosa Sep 19, 2024

RamIdeas Sep 19, 2024

penalosa Sep 19, 2024

penalosa Sep 19, 2024

petebacondarwin commented Sep 19, 2024

penalosa commented Sep 19, 2024

petebacondarwin commented Sep 19, 2024

penalosa commented Sep 19, 2024

RamIdeas Sep 27, 2024

		// @ts-expect-error This is a JS file
		import { getNextMiniflareVersion } from "../../../.github/changeset-version";

Test fixups & skip tests for docs changes #6770

Test fixups & skip tests for docs changes #6770

Conversation

penalosa commented Sep 19, 2024 • edited Loading

What this PR solves / how to test

Author has addressed the following

changeset-bot bot commented Sep 19, 2024 • edited Loading

⚠️ No Changeset found

github-actions bot commented Sep 19, 2024 • edited Loading

penalosa Sep 19, 2024

Choose a reason for hiding this comment

penalosa Sep 19, 2024

Choose a reason for hiding this comment

penalosa Sep 19, 2024

Choose a reason for hiding this comment

RamIdeas Sep 19, 2024

Choose a reason for hiding this comment

penalosa Sep 19, 2024

Choose a reason for hiding this comment

penalosa Sep 19, 2024

Choose a reason for hiding this comment

petebacondarwin commented Sep 19, 2024

penalosa commented Sep 19, 2024

petebacondarwin commented Sep 19, 2024

penalosa commented Sep 19, 2024

RamIdeas Sep 27, 2024

Choose a reason for hiding this comment

penalosa commented Sep 19, 2024 •

edited

Loading

changeset-bot bot commented Sep 19, 2024 •

edited

Loading

github-actions bot commented Sep 19, 2024 •

edited

Loading