Skip to content

Commit

Permalink
tests: composite action for caching, remove fallback restore keys (#4709
Browse files Browse the repository at this point in the history
)

This update refactors the caching steps in the merge-to-main and
full-test-suite workflows by consolidating them into a GitHub composite
action. This change enhances maintainability by reducing code
duplication. Additionally, fallback restore keys have been removed,
ensuring that cache restoration will only occur when an exact match to
the lock file is found, improving cache reliability and avoiding stale
dependencies.

Update:
In this PR, after I moved the restore/save cache logic into composite
actions, I noticed the cache was being restored correctly. However, the
cache hit always returned false. It seems the downstream job doesn’t
have access to the output from the step when it’s part of a composite
action, which is disappointing.

Instead of keeping the the save/restore process split, I reverted to
using the default cache action (which combines it into one) and saves
the cache as a post step when everything completes. The drawback is that
if there’s a failure (such as a test error), the save cache step won’t
execute.

---------

Signed-off-by: Isabel Zimmerman <54685329+isabelizimm@users.noreply.github.com>
Co-authored-by: Jon Vanausdeln <jonv@live.com>
Co-authored-by: sharon <sharon-wang@users.noreply.github.com>
Co-authored-by: Isabel Zimmerman <54685329+isabelizimm@users.noreply.github.com>
Co-authored-by: Wasim Lorgat <mwlorgat@gmail.com>
Co-authored-by: Jonathan <jonathan@rstudio.com>
Co-authored-by: positron-bot[bot] <173392469+positron-bot[bot]@users.noreply.github.com>
Co-authored-by: Christopher Mead <chrisrmead@comcast.net>
  • Loading branch information
8 people authored Sep 19, 2024
1 parent f4c3d4e commit 6f0875d
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 154 deletions.
36 changes: 36 additions & 0 deletions .github/actions/cache-multi-paths/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
name: "Cache Multiple Directories"
description: "Restores/Saves cache for node_modules, build, extensions, and remote"
runs:
using: "composite"
steps:
- name: Cache node_modules
id: cache-node-modules
uses: actions/cache@v4
with:
path: ./node_modules
key: root-node-modules-v6-${{ runner.os }}-${{ hashFiles('yarn.lock') }}

- name: Cache build
id: cache-build
uses: actions/cache@v4
with:
path: ./build/node_modules
key: build-node-modules-v6-${{ runner.os }}-${{ hashFiles('build/yarn.lock') }}

- name: Cache extensions
id: cache-extensions
uses: actions/cache@v4
with:
path: |
./extensions
./extensions/**/node_modules
key: extensions-v6-${{ runner.os }}-${{ hashFiles('extensions/yarn.lock') }}

- name: Cache remote
id: cache-remote
uses: actions/cache@v4
with:
path: |
./remote/node_modules
./remote/**/node_modules
key: remote-node-modules-v6-${{ runner.os }}-${{ hashFiles('remote/yarn.lock') }}
81 changes: 4 additions & 77 deletions .github/workflows/positron-full-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,46 +54,8 @@ jobs:
role-to-assume: ${{ secrets.QA_AWS_RO_ROLE }}
aws-region: ${{ secrets.QA_AWS_REGION }}

- name: Restore cache for node_modules
id: cache-node-modules
uses: actions/cache/restore@v4
with:
path: node_modules
key: root-node-modules-v2-${{ runner.os }}-${{ hashFiles('yarn.lock') }}
restore-keys: |
root-node-modules-v2-${{ runner.os }}-
- name: Restore cache for build
id: cache-build
uses: actions/cache/restore@v4
with:
path: |
build/node_modules
key: build-node-modules-v3-${{ runner.os }}-${{ hashFiles('build/yarn.lock') }}
restore-keys: |
build-node-modules-v3-${{ runner.os }}-
- name: Restore cache for extensions
id: cache-extensions
uses: actions/cache/restore@v4
with:
path: |
extensions
extensions/**/node_modules
key: extensions-v4-${{ runner.os }}-${{ hashFiles('extensions/yarn.lock') }}
restore-keys: |
extensions-v4-${{ runner.os }}-
- name: Restore cache for remote
id: cache-remote
uses: actions/cache/restore@v4
with:
path: |
remote/node_modules
remote/**/node_modules
key: remote-node-modules-v2-${{ runner.os }}-${{ hashFiles('remote/yarn.lock') }}
restore-keys: |
remote-node-modules-v2-${{ runner.os }}-
- name: Cache node_modules, build, extensions, and remote
uses: ./.github/actions/cache-multi-paths

- name: Execute yarn
env:
Expand All @@ -114,40 +76,6 @@ jobs:
yarn --cwd test/automation install --frozen-lockfile
yarn --cwd test/smoke install --frozen-lockfile
# Note cache-hit will be set to true only when cache hit occurs for the exact key match.
# For a partial key match it will be set to false
- name: Cache root node_modules
if: always() && steps.cache-node-modules.outputs.cache-hit != 'true'
uses: actions/cache/save@v4
with:
path: node_modules
key: root-node-modules-v2-${{ runner.os }}-${{ hashFiles('yarn.lock') }}

- name: Cache build node_modules
if: always() && steps.cache-build.outputs.cache-hit != 'true'
uses: actions/cache/save@v4
with:
path: build/node_modules
key: build-node-modules-v3-${{ runner.os }}-${{ hashFiles('build/yarn.lock') }}

- name: Cache extensions
if: always() && steps.cache-extensions.outputs.cache-hit != 'true'
uses: actions/cache/save@v4
with:
path: |
extensions
extensions/**/node_modules
key: extensions-v3-${{ runner.os }}-${{ hashFiles('extensions/yarn.lock') }}

- name: Cache remote node_modules
if: always() && steps.cache-remote.outputs.cache-hit != 'true'
uses: actions/cache/save@v4
with:
path: |
remote/node_modules
remote/**/node_modules
key: remote-node-modules-v2-${{ runner.os }}-${{ hashFiles('remote/yarn.lock') }}

- name: Compile and Download
run: yarn npm-run-all --max_old_space_size=4095 -lp compile "electron x64" playwright-install download-builtin-extensions

Expand Down Expand Up @@ -232,16 +160,15 @@ jobs:
path: .build/logs/smoke-tests-electron/

slack-notification:
name: 'Send Slack notification'
name: "Send Slack notification"
runs-on: ubuntu-latest
needs: linux
if: ${{ failure() }}
steps:
- name: 'Send Slack notification'
- name: "Send Slack notification"
uses: testlabauto/action-test-results-to-slack@v0.0.2
with:
github_token: ${{ secrets.POSITRON_GITHUB_PAT }}
slack_token: ${{ secrets.SMOKE_TESTS_SLACK_TOKEN }}
slack_channel: C07FR1JNZNJ #positron-test-results channel
suite_name: Positron Full Test Suite

80 changes: 3 additions & 77 deletions .github/workflows/positron-merge-to-main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,46 +56,8 @@ jobs:
role-to-assume: ${{ secrets.QA_AWS_RO_ROLE }}
aws-region: ${{ secrets.QA_AWS_REGION }}

- name: Restore cache for node_modules
id: cache-node-modules
uses: actions/cache/restore@v4
with:
path: node_modules
key: root-node-modules-v2-${{ runner.os }}-${{ hashFiles('yarn.lock') }}
restore-keys: |
root-node-modules-v2-${{ runner.os }}-
- name: Restore cache for build
id: cache-build
uses: actions/cache/restore@v4
with:
path: |
build/node_modules
key: build-node-modules-v3-${{ runner.os }}-${{ hashFiles('build/yarn.lock') }}
restore-keys: |
build-node-modules-v3-${{ runner.os }}-
- name: Restore cache for extensions
id: cache-extensions
uses: actions/cache/restore@v4
with:
path: |
extensions
extensions/**/node_modules
key: extensions-v4-${{ runner.os }}-${{ hashFiles('extensions/yarn.lock') }}
restore-keys: |
extensions-v4-${{ runner.os }}-
- name: Restore cache for remote
id: cache-remote
uses: actions/cache/restore@v4
with:
path: |
remote/node_modules
remote/**/node_modules
key: remote-node-modules-v2-${{ runner.os }}-${{ hashFiles('remote/yarn.lock') }}
restore-keys: |
remote-node-modules-v2-${{ runner.os }}-
- name: Cache node_modules, build, extensions, and remote
uses: ./.github/actions/cache-multi-paths

- name: Execute yarn
env:
Expand All @@ -116,41 +78,6 @@ jobs:
yarn --cwd test/automation install --frozen-lockfile
yarn --cwd test/smoke install --frozen-lockfile
# Note cache-hit will be set to true only when cache hit occurs for the exact key match.
# For a partial key match it will be set to false
- name: Cache root node_modules
if: always() && steps.cache-node-modules.outputs.cache-hit != 'true'
uses: actions/cache/save@v4
with:
path: node_modules
key: root-node-modules-v2-${{ runner.os }}-${{ hashFiles('yarn.lock') }}

- name: Cache build node_modules
if: always() && steps.cache-build.outputs.cache-hit != 'true'
uses: actions/cache/save@v4
with:
path: build/node_modules
key: build-node-modules-v3-${{ runner.os }}-${{ hashFiles('build/yarn.lock') }}

- name: Cache extensions
if: always() && steps.cache-extensions.outputs.cache-hit != 'true'
uses: actions/cache/save@v4
with:
path: |
extensions
extensions/**/node_modules
key: extensions-v4-${{ runner.os }}-${{ hashFiles('extensions/yarn.lock') }}

- name: Cache remote node_modules
if: always() && steps.cache-remote.outputs.cache-hit != 'true'
uses: actions/cache/save@v4
with:
path: |
remote/node_modules
remote/**/node_modules
key: remote-node-modules-v2-${{ runner.os }}-${{ hashFiles('remote/yarn.lock') }}

- name: Compile and Download
run: yarn npm-run-all --max_old_space_size=4095 -lp compile "electron x64" playwright-install download-builtin-extensions

Expand Down Expand Up @@ -212,7 +139,7 @@ jobs:
target: ${{ env.SMOKETEST_TARGET }}

slack-notification:
name: 'Send Slack notification'
name: "Send Slack notification"
runs-on: ubuntu-latest
needs: linux
if: ${{ failure() && needs.linux.outputs.target == 'smoketest-merge-to-main' }}
Expand All @@ -224,4 +151,3 @@ jobs:
slack_token: ${{ secrets.SMOKE_TESTS_SLACK_TOKEN }}
slack_channel: C07FR1JNZNJ #positron-test-results channel
suite_name: Positron Merge to Main Test Suite

0 comments on commit 6f0875d

Please sign in to comment.