GH-641: Fix Known CI Issues#658
Conversation
…me and replace `::set-output ` with `$GITHUB_OUTPUT`
…ng for node_modules
…ws and adjust PHP test
.github/workflows/test-measure.yml
Outdated
| - name: Get node version | ||
| id: node | ||
| run: echo "version=$(node -v)" >> $GITHUB_OUTPUT | ||
|
|
||
| - name: Cache node_modules | ||
| id: node_modules | ||
| uses: actions/cache@v5.0.3 | ||
| with: | ||
| path: '**/node_modules' | ||
| key: ${{ runner.os }}-node-modules-${{ hashFiles('**/package-lock.json') }}-${{ steps.node.outputs.version }} | ||
|
|
There was a problem hiding this comment.
This is being added multiple times in the file, can this be converted to an action and re-used everywhere?
There was a problem hiding this comment.
.github/workflows/test-measure.yml
Outdated
| uses: actions/cache@v5.0.3 | ||
| with: | ||
| path: '**/node_modules' | ||
| key: ${{ runner.os }}-node-modules-${{ hashFiles('**/package-lock.json') }}-${{ steps.node.outputs.version }} |
There was a problem hiding this comment.
The hashing should also depend on npmrc
.github/workflows/test-measure.yml
Outdated
| key: ${{ runner.os }}-node-modules-${{ hashFiles('**/package-lock.json') }}-${{ steps.node.outputs.version }} | ||
|
|
||
| - name: Install Node dependencies | ||
| if: needs.pre-run.outputs.changed-php-count > 0 && steps.node_modules.outputs.cache-hit != 'true' |
There was a problem hiding this comment.
Why are node dependencies dependent on needs.pre-run.outputs.changed-php-count?
There was a problem hiding this comment.
The condition needs.pre-run.outputs.changed-php-count > 0 is added to avoid unnecessary Node installation when PHP tests are not required.
In this workflow, the unit-test-php job cannot be skipped entirely due to required status checks, so it still runs even when no PHP files are changed. Because of that, without this condition:
The cache step would still execute
And on a cache miss, npm ci would run
This would install Node dependencies even though PHP tests are not needed
So the condition ensures that Node-related setup (cache + install) only runs when PHP files have actually changed, preventing unnecessary installs caused by cache misses in irrelevant runs.
As per the latest change, this has been removed.
.github/workflows/test-measure.yml
Outdated
| key: ${{ runner.os }}-node-modules-${{ hashFiles('**/package-lock.json') }}-${{ steps.node.outputs.version }} | ||
|
|
||
| - name: Install Node dependencies | ||
| if: needs.pre-run.outputs.changed-php-count > 0 && steps.node_modules.outputs.cache-hit != 'true' |
There was a problem hiding this comment.
Convert steps.node_modules.outputs.cache-hit != 'true' to a variable and re-use
SHOULD_INSTALL: ${{ steps.node_modules.outputs.cache-hit != 'true' }}
2 participants
Description
Resolves multiple issues in the CI workflow that were causing incorrect behavior and deprecation warnings.
Fixes: #641
Technical Details
Replaced all deprecated ::set-output usages with the recommended $GITHUB_OUTPUT approach to ensure compatibility with current GitHub Actions standards.
Eliminated hard-coded "CK theme" references in log messages. These have been replaced with dynamic values using ${{ github.event.repository.name }}, improving reusability across repositories.
Corrected the logic of the Notice step in the unit-test-php job. Previously, it executed when PHP files were modified; it now correctly runs only when no PHP changes are detected.
Introduced dependency caching for node_modules using actions/cache@v5.0.3
The cache key is composed of:
package-lock.jsonhashChecklist
::set-outputsyntax → replaced with$GITHUB_OUTPUTactions/cachefornode_modulesScreenshots