Skip to content
Merged
Show file tree
Hide file tree
Changes from 47 commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
c72e794
Actions: bundle size and npm scripts
vmarta Apr 9, 2025
b4703b4
Tweak condition
vmarta Apr 9, 2025
d5d5cc1
Lighthouse test
vmarta Apr 10, 2025
fc57780
Fix test workflow
vmarta Apr 10, 2025
a2683da
Try different way to set env variable
vmarta Apr 10, 2025
32a38ea
Aargh, forgot to close the quote
vmarta Apr 10, 2025
651ad64
Comment out lighthouse test for now
vmarta Apr 10, 2025
b6e961b
Move to separate job those that don't test code correctness
vmarta Apr 10, 2025
42ed6d8
Refactor the MRT's node version
vmarta Apr 10, 2025
cdf9b69
Try fixing error
vmarta Apr 10, 2025
4f982ca
Try fixing error
vmarta Apr 10, 2025
3a50d09
Experiment with `bundlesize`
vmarta Apr 10, 2025
024c0a8
Check bundle sizes
vmarta Apr 10, 2025
565fefd
Simplify code with github.action_path
vmarta Apr 10, 2025
8728f09
Debugging
vmarta Apr 10, 2025
08a0ff4
Workaround for bug with bundlesize
vmarta Apr 10, 2025
6e64b34
Lighthouse test now accepts config filename to be more reusable
vmarta Apr 11, 2025
e35e4aa
Experiment with passing in json config
vmarta Apr 11, 2025
1158ef2
Experiment with passing in json config
vmarta Apr 11, 2025
a2c1732
For the other jobs, run the npm-scripts test
vmarta Apr 11, 2025
e03a9dd
Add comment to clarify
vmarta Apr 11, 2025
74116b7
Remove duplicate test
vmarta Apr 11, 2025
643927b
Revert "Remove duplicate test"
vmarta Apr 11, 2025
2e98294
Smoke testing npm scripts now run in all nodes and OSes
vmarta Apr 12, 2025
b92ccff
Delete no-longer-needed code
vmarta Apr 12, 2025
757aaa6
Test running an invalid command
vmarta Apr 12, 2025
98d9429
Test running an invalid command
vmarta Apr 12, 2025
d50e055
Move more things to setup_ubuntu and setup_windows
vmarta Apr 14, 2025
6803ca5
Move more things to setup_ubuntu and setup_windows
vmarta Apr 14, 2025
570222d
Move more things to setup_ubuntu and setup_windows
vmarta Apr 14, 2025
227a949
Debugging
vmarta Apr 14, 2025
3f43a4b
Debugging
vmarta Apr 14, 2025
3c89a66
Debugging
vmarta Apr 14, 2025
5e9ca4f
Debugging
vmarta Apr 14, 2025
0ed508a
Debugging
vmarta Apr 14, 2025
bbb6698
Debugging
vmarta Apr 14, 2025
fc852ce
Debugging
vmarta Apr 14, 2025
c9d22b2
Debugging
vmarta Apr 14, 2025
56f37c6
New step to verify Node and NPM versions
vmarta Apr 14, 2025
b3644eb
Debugging windows
vmarta Apr 14, 2025
81b990a
Some refactoring
vmarta Apr 14, 2025
bbd7914
Prints clearer message
vmarta Apr 14, 2025
bfe4f06
Clean up code
vmarta Apr 15, 2025
1be0723
A bit of reordering
vmarta Apr 15, 2025
f27c50f
Merge branch 'feature/extensibility-v2' into vm/bring-back-github-act…
vmarta Apr 21, 2025
a7d1eab
Add comments
vmarta Apr 21, 2025
f096f6f
Add comments
vmarta Apr 21, 2025
cfdc725
Add error handling for jq
vmarta Apr 22, 2025
5711449
Extract the app generation into its own action
vmarta Apr 22, 2025
a962651
Fix the error handling
vmarta Apr 22, 2025
33a8166
Fix action
vmarta Apr 22, 2025
8444352
The timeout-minutes cannot be used inside an action
vmarta Apr 22, 2025
ea6b108
Remove console log
vmarta Apr 24, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 29 additions & 3 deletions .github/actions/bundle_size_test/action.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,34 @@
name: bundlesize
name: bundle_size_test
inputs:
cwd:
required: true
config:
required: true
runs:
using: composite
steps:
- name: Check bundle size
- name: Analyze build
working-directory: ${{ inputs.cwd }}
run: |-
npm run test:max-file-size
npm run analyze-build
shell: bash

- name: Check bundle sizes
working-directory: ${{ inputs.cwd }}
# NOTE: A bug prevented us from running bundlesize with a config file:
# npx bundlesize --scope ${{ github.action_path }}/bundlesize.config.json
# For more details, see https://github.com/siddharthkp/bundlesize/issues/326
run: |-
json_string='${{ inputs.config }}'

# Parse the given json string and build the command to run
echo $json_string | jq -r 'to_entries[] | "npx bundlesize -f \"" + .key + "\" -s " + .value' | while read -r command; do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add some error handling in the jq parsing and print a message to the command failed?

# Example of command: npx bundlesize -f "build/main.js" -s 10kB
eval $command
done
shell: bash

- name: Report bundle sizes
run: |-
node ./scripts/report-bundle-size.js ${{ inputs.cwd }}/build
shell: bash
2 changes: 2 additions & 0 deletions .github/actions/datadog/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@ runs:
steps:
- name: Send metrics to Datadog
run : |
# TODO: move it back to setup_ubuntu action
# For the datadog cli, it must be installed via python
# to install python packages on CI environment, we must activate the virtual env
# or otherwise it throws error: externally-managed-environment
python3 -m venv venv
source venv/bin/activate
pip install datadog

# TODO: move it back to setup_ubuntu action
# Add a dogrc so we can submit metrics to datadog
printf "[Connection]\napikey = ${{inputs.datadog_api_key}}\nappkey =\n" > ~/.dogrc

Expand Down
16 changes: 13 additions & 3 deletions .github/actions/lighthouse_ci/action.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,18 @@
name: lighthouse_ci
inputs:
project_dir:
required: true
config_filename:
required: true
runs:
using: composite
steps:
- name: Run Lighthouse CI on the PWA
# This test is flaky, so let's try it three times!
run: npm run test:lighthouse --prefix packages/template-retail-react-app || npm run test:lighthouse --prefix packages/template-retail-react-app || npm run test:lighthouse --prefix packages/template-retail-react-app
- name: Run Lighthouse CI
working-directory: ${{ inputs.project_dir }}
run: |-
lighthouse_test="npx @lhci/cli autorun --config=${{ github.action_path }}/${{ inputs.config_filename }}"
# This test is flaky, so let's try it three times!
$lighthouse_test || $lighthouse_test || $lighthouse_test
shell: bash
env:
NODE_ENV: production
38 changes: 38 additions & 0 deletions .github/actions/lighthouse_ci/lighthouserc.retail-react-app.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Copyright (c) 2021, salesforce.com, inc.
* All rights reserved.
* SPDX-License-Identifier: BSD-3-Clause
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/

module.exports = {
// The different Lighthouse CI configuration options:
// https://github.com/GoogleChrome/lighthouse-ci/blob/master/docs/configuration.md
ci: {
collect: {
startServerCommand: 'npm run start',
//NOTE: Adjust the key pages URLs that you find important to your website.
url: [
'http://localhost:3000/',
'http://localhost:3000/global/en-GB/category/womens',
'http://localhost:3000/global/en-GB/product/25493613M',
'http://localhost:3000/global/en-GB/search?q=suit'
],
startServerReadyPattern: 'First build complete',
startServerReadyTimeout: 90000
},
upload: {
target: 'temporary-public-storage'
},
assert: {
aggregationMethod: 'median',
assertions: {
// NOTE: Adjust the scores accordingly as the performance is improved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious: Does the CI fail if these scores aren't met or do we just report the results and continue ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the lines below, we chose to use error (instead of warn), so CI will fail. And it will also report the result and continue because the lighthouse test runs in a separate job.

'categories:performance': ['error', {minScore: 0.3}],
'categories:pwa': ['error', {minScore: 0.9}],
'categories:seo': ['error', {minScore: 0.85}],
'categories:accessibility': ['error', {minScore: 0.88}]
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
name: smoke_tests
name: npm_scripts
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renaming this CI action to be clearer that we're actually smoke testing the npm scripts (and not retail-react-app site).

inputs:
dir:
required: false
# The path to a project to test
default: "./packages/template-retail-react-app"
cwd:
required: true
runs:
using: composite
steps:
- name: Smoke test scripts
- name: Smoke test the npm scripts
run: |-
# Basic smoke-tests for uncommonly run scripts in a project
node ./scripts/smoke-test-npm-scripts.js --dir "${{ inputs.dir }}"
node ./scripts/smoke-test-npm-scripts.js --dir "${{ inputs.cwd }}"
shell: bash
35 changes: 27 additions & 8 deletions .github/actions/setup_ubuntu/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,46 @@ inputs:
cwd:
required: false
default: ${{ github.workspace }}
node:
required: true
description: Major node version
npm:
required: false
description: Major npm version
update_npm:
required: false
default: false
description: "Setup Ubuntu Machine"
runs:
using: composite
steps:
- name: Setup Node
uses: actions/setup-node@v4
with:
node-version: ${{ inputs.node }}
cache: npm

- name: Update NPM version
if: inputs.update_npm == 'true'
run: |-
npm install -g npm@${{ inputs.npm }}
shell: bash

- name: Verify node and npm versions
uses: ./.github/actions/verify_node_npm_major_versions
with:
node: ${{ inputs.node }}
npm: ${{ inputs.npm }}

- name: Install Dependencies
working-directory: ${{ inputs.cwd }}
run: |-
# Install node dependencies
node ./scripts/gtime.js monorepo_install npm ci

# Build the PWA
# npm run lerna -- run analyze-build --scope "@salesforce/retail-react-app"

# Report bundle sizes
# node ./scripts/report-bundle-size.js
Comment on lines -16 to -20
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bundle-related stuffs are moved to their own action file.

Focusing this setup_ubuntu action for mainly setting up node and npm. So that the action is more generic and reusable.


# Install Snyk CLI
# TODO: Ticket W-12425059. Revisit Snyk CLI integration to monitor manifest files on generated projects.
# TODO: Latest Snyk CLI version is currently failing on npm i. We use the alternative Snyk GitHub integration.
# sudo npm install -g snyk

# Install Lighthouse CI CLI
sudo npm install -g @lhci/cli
shell: bash
27 changes: 27 additions & 0 deletions .github/actions/setup_windows/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,37 @@ inputs:
cwd:
required: false
default: "${PWD}"
node:
required: true
description: Major node version
npm:
required: false
description: Major npm version
update_npm:
required: false
default: false
description: "Setup Windows Machine"
runs:
using: composite
steps:
- name: Setup Node
uses: actions/setup-node@v4
with:
node-version: ${{ inputs.node }}
cache: npm

- name: Update NPM version
if: inputs.update_npm == 'true'
run: |-
npm install -g npm@${{ inputs.npm }}
shell: bash

- name: Verify node and npm versions
uses: ./.github/actions/verify_node_npm_major_versions
with:
node: ${{ inputs.node }}
npm: ${{ inputs.npm }}

- name: Install Dependencies
run: |-
# Install node dependencies
Expand Down
39 changes: 39 additions & 0 deletions .github/actions/verify_node_npm_major_versions/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
name: verify_node_npm_major_versions
inputs:
node:
required: false
description: Major node version
npm:
required: false
description: Major npm version
runs:
using: composite
steps:
- run: |-
EXPECTED_NODE_VERSION="${{ inputs.node }}"
EXPECTED_NPM_VERSION="${{ inputs.npm }}"

FULL_NODE_VERSION=$(node -v)
FULL_NPM_VERSION=$(npm -v)
# Parse out the _major_ versions
INSTALLED_NODE_VERSION=$(echo $FULL_NODE_VERSION | sed 's/^v\([0-9]*\).*/\1/')
INSTALLED_NPM_VERSION=$(echo $FULL_NPM_VERSION | sed 's/^\([0-9]*\).*/\1/')

if [ -n "$EXPECTED_NODE_VERSION" ]; then
if [ "$INSTALLED_NODE_VERSION" != "$EXPECTED_NODE_VERSION" ]; then
echo "The node version is incorrect. Expected: v$EXPECTED_NODE_VERSION, but found: v$INSTALLED_NODE_VERSION"
exit 1
fi
fi

if [ -n "$EXPECTED_NPM_VERSION" ]; then
if [ "$INSTALLED_NPM_VERSION" != "$EXPECTED_NPM_VERSION" ]; then
echo "The npm version is incorrect. Expected: $EXPECTED_NPM_VERSION, but found: $INSTALLED_NPM_VERSION"
exit 1
fi
fi

echo "Node/npm version check completed successfully."
echo "Installed node version: $FULL_NODE_VERSION"
echo "Installed npm version: $FULL_NPM_VERSION"
shell: bash
Loading
Loading