Skip to content

Commit 737e0eb

Browse files
davidcavazosglasnt
andauthored
fix: remove experimental from forked repo tests (#4088)
* feat: remove experimental from forked repo tests * mark packages to run tests * mark old tests as legacy * add more comments * add instructions to sunset old tests * rename workflow back * remove experimental from created checks * remove packages to trigger tests * experiment to see if check fulfills required tests * add permissions to experiment * rename lint back * remove experiment * only run tests on non-forks * remove experimental * document ci testing * add legacy to old dev tests * rename summary to Custard CI / test * reorder list * Update CONTRIBUTING.md Co-authored-by: Katie McLaughlin <[email protected]> * add note to pull request template --------- Co-authored-by: Katie McLaughlin <[email protected]>
1 parent b2f99d8 commit 737e0eb

File tree

6 files changed

+49
-39
lines changed

6 files changed

+49
-39
lines changed

.github/PULL_REQUEST_TEMPLATE.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,13 @@ Note: Before submitting a pull request, please open an issue for discussion if y
88
- [ ] I have followed guidelines from [CONTRIBUTING.MD](https://github.com/GoogleCloudPlatform/nodejs-docs-samples/blob/main/CONTRIBUTING.md) and [Samples Style Guide](https://googlecloudplatform.github.io/samples-style-guide/)
99
- [ ] **Tests** pass: `npm test` (see [Testing](https://github.com/GoogleCloudPlatform/nodejs-docs-samples/blob/main/CONTRIBUTING.md#run-the-tests-for-a-single-sample))
1010
- [ ] **Lint** pass: `npm run lint` (see [Style](https://github.com/GoogleCloudPlatform/nodejs-docs-samples/blob/main/CONTRIBUTING.md#style))
11+
- [ ] **Required CI tests** pass (see [CI testing](https://github.com/GoogleCloudPlatform/nodejs-docs-samples/blob/main/CONTRIBUTING.md#ci-testing))
1112
- [ ] These samples need a new **API enabled** in testing projects to pass (let us know which ones)
1213
- [ ] These samples need a new/updated **env vars** in testing projects set to pass (let us know which ones)
1314
- [ ] This pull request is from a branch created directly off of `GoogleCloudPlatform/nodejs-docs-samples`. Not a fork.
1415
- [ ] This sample adds a new sample directory, and I updated the [CODEOWNERS file](https://github.com/GoogleCloudPlatform/nodejs-docs-samples/blob/main/CODEOWNERS) with the codeowners for this sample
1516
- [ ] This sample adds a new sample directory, and I created [GitHub Actions workflow](https://github.com/GoogleCloudPlatform/nodejs-docs-samples/blob/main/CONTRIBUTING.md#adding-new-samples) for this sample
1617
- [ ] This sample adds a new **Product API**, and I updated the [Blunderbuss issue/PR auto-assigner](https://github.com/GoogleCloudPlatform/nodejs-docs-samples/blob/main/.github/blunderbuss.yml) with the codeowners for this sample
1718
- [ ] Please **merge** this PR for me once it is approved
19+
20+
> **Note**: Any check with `(dev)`, `(experimental)`, or `(legacy)` can be ignored and should **not block** your PR from merging (see [CI testing](https://github.com/GoogleCloudPlatform/nodejs-docs-samples/blob/main/CONTRIBUTING.md#ci-testing)).

.github/workflows/custard-ci-dev.yaml

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15-
name: Custard CI (dev)
15+
name: (legacy dev) Custard CI
1616
on:
1717
push:
1818
branches:
@@ -27,7 +27,7 @@ env:
2727

2828
jobs:
2929
affected:
30-
name: Finding affected tests
30+
name: (legacy) Finding affected tests
3131
runs-on: ubuntu-latest
3232
timeout-minutes: 2
3333
outputs:
@@ -61,7 +61,8 @@ jobs:
6161
echo "setups=$(./cloud-samples-tools/bin/custard setup-files .github/config/nodejs-dev.jsonc paths.txt)" >> $GITHUB_OUTPUT
6262
6363
nodejs-test:
64-
name: test
64+
if: github.event.pull_request && github.event.pull_request.head.repo.fork == false
65+
name: (legacy) test
6566
needs: affected
6667
runs-on: ubuntu-latest
6768
timeout-minutes: 120 # 2 hours hard limit

.github/workflows/custard-ci.yaml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15+
# IMPORTANT: DO NOT CHANGE THE NAME OF THIS WORKFLOW!
16+
# The workflow named "Custard CI" triggers both custard-run.yaml and custard-run-dev.yaml.
17+
# TODO: To sunset old tests remove `affected`, `lint`, and `test` jobs (only keep `region-tags`).
1518
name: Custard CI
1619
on:
1720
push:
@@ -28,7 +31,7 @@ env:
2831
# TODO: remove all jobs except region-tags (should be tested by custard-run workflows)
2932
jobs:
3033
affected:
31-
name: Finding affected tests
34+
name: (legacy) Finding affected tests
3235
runs-on: ubuntu-latest
3336
timeout-minutes: 2
3437
outputs:
@@ -119,6 +122,8 @@ jobs:
119122
- run: ./.github/workflows/utils/region-tags-tests.sh
120123

121124
test:
125+
if: github.event.pull_request && github.event.pull_request.head.repo.fork == false
126+
name: (legacy) test
122127
needs: affected
123128
runs-on: ubuntu-latest
124129
timeout-minutes: 120 # 2 hours hard limit

.github/workflows/custard-run-dev.yaml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15-
name: (experimental / dev) Custard run
15+
name: (dev) Custard run
1616

1717
on:
1818
# Run tests when a pull request is created or updated.
@@ -52,7 +52,7 @@ jobs:
5252
head-sha: ${{ github.event.workflow_run.head_sha || inputs.ref || github.sha }}
5353
config-file: .github/config/nodejs-dev.jsonc
5454
paths: ${{ (inputs.run-all && '.') || inputs.paths || '' }}
55-
check-name: (experimental / dev) Custard CI
55+
check-name: (dev) Custard CI / tests
5656
create-check-if: ${{ !!github.event.workflow_run }}
5757

5858
test:
@@ -77,7 +77,7 @@ jobs:
7777
id: queued
7878
with:
7979
sha: ${{ github.event.workflow_run.head_sha || inputs.ref || github.sha }}
80-
name: (experimental / dev) Custard CI / ${{ github.job }} (${{ matrix.path }})
80+
name: (dev) Custard CI / ${{ github.job }} (${{ matrix.path }})
8181
job-name: ${{ github.job }} (${{ matrix.path }})
8282
if: ${{ !!github.event.workflow_run }}
8383
- name: Checkout

.github/workflows/custard-run.yaml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15-
name: (experimental) Custard run
15+
name: Custard run
1616

1717
on:
1818
# Run tests when a pull request is created or updated.
@@ -57,7 +57,7 @@ jobs:
5757
head-sha: ${{ github.event.workflow_run.head_sha || inputs.ref || github.sha }}
5858
config-file: .github/config/nodejs.jsonc
5959
paths: ${{ (inputs.run-all && '.') || inputs.paths || '' }}
60-
check-name: (experimental) Custard CI
60+
check-name: Custard CI / tests
6161
create-check-if: ${{ !!github.event.workflow_run }}
6262

6363
lint:
@@ -73,7 +73,7 @@ jobs:
7373
with:
7474
sha: ${{ github.event.workflow_run.head_sha || inputs.ref || github.sha }}
7575
status: in_progress
76-
name: (experimental) Custard CI / ${{ github.job }}
76+
name: Custard CI / ${{ github.job }}
7777
if: ${{ !!github.event.workflow_run }}
7878
- name: Checkout
7979
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4
@@ -123,7 +123,7 @@ jobs:
123123
id: queued
124124
with:
125125
sha: ${{ github.event.workflow_run.head_sha || inputs.ref || github.sha }}
126-
name: (experimental) Custard CI / ${{ github.job }} (${{ matrix.path }})
126+
name: Custard CI / ${{ github.job }} (${{ matrix.path }})
127127
job-name: ${{ github.job }} (${{ matrix.path }})
128128
if: ${{ !!github.event.workflow_run }}
129129
- name: Checkout

CONTRIBUTING.md

Lines changed: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -64,34 +64,35 @@ const assert = require('node:assert/strict');
6464

6565
### CI testing
6666

67-
For new samples, a GitHub Actions workflow should be created to run your tests
68-
on the CI system:
69-
70-
1. Check that your new samples and sample tests are on a branch created directly from this repo `GoogleCloudPlatform/nodejs-docs-samples`. Not a fork.
71-
72-
1. Add an entry to
73-
[.github/workflows/utils/workflows.json](https://github.com/GoogleCloudPlatform/nodejs-docs-samples/blob/main/.github/workflows/utils/workflows.json)
74-
matching the directory with your sample code.
75-
76-
1. From the root of the repo, generate a new workflow in the
77-
[workflows](https://github.com/GoogleCloudPlatform/nodejs-docs-samples/blob/main/.github/workflows)
78-
directory. You can specify a `path` to only generate the specific workflow,
79-
e.g. `cloud-tasks`. If the path is omitted, all workflows will be generated.
80-
81-
node .github/workflows/utils/generate.js -s [path]
82-
83-
> **Note** There are some existing samples that use an alternative CI system. It
84-
> is recommended to use GitHub Actions for new samples, but these instructions
85-
> are provided below for your reference.
86-
>
87-
> Add a **build configuration file (`.cfg`)** for your samples in
88-
> **[`.kokoro/`](https://github.com/GoogleCloudPlatform/nodejs-docs-samples/tree/main/.kokoro)**.
89-
> Check existing config files for the right format.
90-
>
91-
> All tests need a corresponding job file outside of GitHub. If you are a
92-
> Googler, please provide the CL alongside your PR. See the internal codelab for
93-
> Kokoro for details. If you don't work at Google, the person reviewing your PR
94-
> will create the job config for you.
67+
> **tl;dr**: Any check with `(dev)`, `(experimental)`, or `(legacy)` can be ignored and should **not block** your PR from merging.
68+
69+
This repository uses the 🍮 **Custard CI** from
70+
[`GoogleCloudPlatform/cloud-samples-tools`](https://github.com/GoogleCloudPlatform/cloud-samples-tools)
71+
to set up and run tests.
72+
73+
First, it checks which files changed in a PR to find which packages were affected.
74+
In this repo, a _package_ is defined as a directory containing a `package.json` file.
75+
76+
If a global file (not belonging to a package) is changed, all packages are marked as affected.
77+
Global files are can contain repo-level configurations that could affect other packages.
78+
Typically, contributors should only modify files in a package.
79+
80+
Only affected packages are linted and tested.
81+
For tests, we use a
82+
[matrix strategy](https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/running-variations-of-jobs-in-a-workflow)
83+
to generate isolated jobs for each affected test.
84+
85+
We're working to improve the testing infrastructure, while keeping tests stable.
86+
This means you'll sometimes see some new experimental tests, you can generally ignore them.
87+
Or two versions of one test running while we make sure the new version works well.
88+
89+
Here's a list of which tests are required and which ones you can ignore.
90+
* `Custard CI / lint`: (**required**) runs only for affected packages
91+
* `Custard CI / tests`: (**required**) this runs until all tests have finished
92+
* `Custard CI / test (package)`: (**required**) runs only for affected packages
93+
* `(dev) Custard CI / test (package)`: (_ignore_) this test has errors we're working on
94+
* `(experimental) Custard CI / ...`: (_ignore_) this is a new unstable version
95+
* `Custard CI / (legacy) ...`: (_ignore_) this is the old version running while we make sure the new one works
9596

9697
### TypeScript Support
9798

0 commit comments

Comments
 (0)