Skip to content

Commit b4f3a6f

Browse files
pattonwebzclaude
andcommitted
Fix three code review issues in workflow caching
node_modules cache + npm ci: npm ci deletes node_modules before reinstalling, so restoring the cache and then running npm ci was a no-op — the restored directory was thrown away immediately. Fix by adding id: cache-node-modules to each cache step and guarding the install with if: cache-hit != 'true'. On an exact key match the install is skipped entirely; on a partial or missing match npm ci runs normally and saves the result. Affects: jest-tests.yml, lint-js.yml, make-pot.yml, build-plugin-with-ref.yml (replaced the manual [ -d node_modules ] shell check with the proper cache-hit output). npm config set ignore-scripts true: this flag persists for the remainder of the job, disabling lifecycle scripts in later steps (e.g. Cypress setup). Replaced with the per-command --ignore-scripts flag on npm ci in jest-tests.yml and copilot-setup-steps.yml. WP test library cached under 'latest' key: install-wp-tests.sh returns immediately when /tmp/wordpress already exists, so a cached 'latest' run can keep serving an older WordPress release after upstream ships a new one. Added if: matrix.wp_version != 'latest' to skip caching for the three matrix jobs that use the moving target; the pinned 6.2 job continues to benefit from the cache. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent ab03cc5 commit b4f3a6f

6 files changed

Lines changed: 12 additions & 17 deletions

File tree

.github/workflows/build-plugin-with-ref.yml

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ jobs:
8181
cache-dependency-path: package-lock.json
8282

8383
- name: Cache node_modules
84+
id: cache-node-modules
8485
uses: actions/cache@v4
8586
with:
8687
path: node_modules
@@ -108,16 +109,8 @@ jobs:
108109
if [ -f composer.json ]; then composer install --no-dev --prefer-dist --prefer-offline --no-progress --no-interaction; else echo "No composer.json"; fi
109110
110111
- name: Install npm dependencies
111-
run: |
112-
if [ -f package.json ]; then
113-
if [ -d node_modules ]; then
114-
echo "node_modules cache hit, skipping install"
115-
else
116-
npm ci --prefer-offline --no-audit
117-
fi
118-
else
119-
echo "No package.json"
120-
fi
112+
if: steps.cache-node-modules.outputs.cache-hit != 'true' && hashFiles('package.json') != ''
113+
run: npm ci --prefer-offline --no-audit
121114

122115
- name: Extract plugin version and commit hash
123116
id: version

.github/workflows/copilot-setup-steps.yml

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,7 @@ jobs:
6464
${{ runner.os }}-docker-
6565
6666
- name: Install npm dependencies
67-
run: |
68-
# Skip postinstall scripts to avoid Cypress download issues in CI
69-
npm config set ignore-scripts true
70-
npm ci --prefer-offline --no-audit
67+
run: npm ci --ignore-scripts --prefer-offline --no-audit
7168

7269
- name: Install Composer dependencies
7370
run: |

.github/workflows/jest-tests.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ jobs:
2727
${{ runner.os }}-node-
2828
2929
- name: Cache node_modules
30+
id: cache-node-modules
3031
uses: actions/cache@v4
3132
with:
3233
path: node_modules
@@ -35,9 +36,8 @@ jobs:
3536
${{ runner.os }}-node_modules-
3637
3738
- name: Install dependencies (skipping postinstall)
38-
run: |
39-
npm config set ignore-scripts true
40-
npm ci --prefer-offline --no-audit
39+
if: steps.cache-node-modules.outputs.cache-hit != 'true'
40+
run: npm ci --prefer-offline --no-audit --ignore-scripts
4141

4242
- name: Run Jest tests
4343
run: npm run test:jest

.github/workflows/lint-js.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ jobs:
2929
uses: actions/checkout@v3
3030

3131
- name: Cache Node.js modules
32+
id: cache-node-modules
3233
uses: actions/cache@v4
3334
with:
3435
path: node_modules
@@ -37,6 +38,7 @@ jobs:
3738
${{ runner.os }}-node_modules-
3839
# The lint stage doesn't run the unit tests or use code style, so no need for PHPUnit, WPCS or phpcompatibility.
3940
- name: 'Install NPM packages'
41+
if: steps.cache-node-modules.outputs.cache-hit != 'true'
4042
run: npm ci --prefer-offline --no-audit --ignore-scripts
4143

4244
- name: Get only files changed in this PR

.github/workflows/make-pot.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ jobs:
4545
node-version: '20'
4646

4747
- name: Cache Node.js modules
48+
id: cache-node-modules
4849
uses: actions/cache@v4
4950
with:
5051
path: node_modules
@@ -53,6 +54,7 @@ jobs:
5354
${{ runner.os }}-node_modules-
5455
5556
- name: Install dependencies
57+
if: steps.cache-node-modules.outputs.cache-hit != 'true'
5658
run: npm ci --prefer-offline --no-audit --ignore-scripts
5759

5860
- name: Build plugin (dotorg dist)

.github/workflows/phpunit.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ jobs:
9494
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
9595

9696
- name: Cache WordPress test library
97+
if: matrix.wp_version != 'latest'
9798
uses: actions/cache@v4
9899
with:
99100
path: |

0 commit comments

Comments
 (0)