-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
336-feat: configure workflow for lighthouse to run on deployments #351
Conversation
WalkthroughThe changes introduce automated Lighthouse testing in the CI/CD pipeline for PR deployments. This is achieved through the addition of a GitHub Actions workflow ( Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Qodana for JSIt seems all right 👌 No new problems were found according to the checks applied 💡 Qodana analysis was run in the pull request mode: only the changed files were checked Contact Qodana teamContact us at [email protected]
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- .github/workflows/lighthouse.yml (1 hunks)
- lighthouserc.json (1 hunks)
Files skipped from review due to trivial changes (1)
- lighthouserc.json
Additional comments not posted (5)
.github/workflows/lighthouse.yml (5)
3-6
: Workflow Trigger Configuration ApprovedThe configuration to trigger the workflow on a
repository_dispatch
event with typepreview-created
aligns well with the objectives to run tests on PR deployments.
10-20
: Approve Node Setup and Dependency InstallationUsing Node.js version 20 and installing Lighthouse CLI are appropriate for running the tests. Consider pinning the exact version of Node.js and Lighthouse CLI for more stable and predictable builds.
22-24
: Dynamic URL Construction for Lighthouse Tests ApprovedConstructing the URL dynamically to target the specific PR deployment is a smart approach and aligns perfectly with the PR objectives.
48-59
: Posting Lighthouse Results to PR ApprovedAutomating the posting of Lighthouse test results directly in the PR comments enhances visibility and aligns with the objectives. This is a good use of GitHub Actions to automate communication.
25-47
: Artifact Upload and Report Generation Steps ApprovedUploading Lighthouse artifacts and generating a detailed report summary are well-implemented. Verify the paths and JSON structure used in the jq commands to ensure they are correct and will not lead to runtime errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/lighthouse.yml (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- .github/workflows/lighthouse.yml
- name: Setup Node | ||
uses: actions/setup-node@v4 | ||
with: | ||
node-version: '20.x' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the NODE_VERSION
env variable as in other workflows
node-version: '20.x' | ||
|
||
- name: Install dependencies | ||
run: npm install && npm install -g @lhci/[email protected] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd use bahmutov/npm-install@v1
github action
run: npm install && npm install -g @lhci/[email protected] | ||
|
||
- name: Run Lighthouse | ||
run: lhci autorun --collect.url=https://pr${{ github.event.client_payload.number }}.rs.school |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can I use npx lhci
here and remove npm install -g @lhci/[email protected]
from the previos step?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried and there is no benefits, workflow execution time becomes even longer. Same with npm-install action.
And the method used here is recommended from lighthouse docs.
What type of PR is this? (select all that apply)
Description
As github docs says, repository_dispatch works only for files in main branch, so seems like i can't trigger it without merging pr.
Related Tickets & Documents
Screenshots, Recordings
Added/updated tests?
[optional] What gif best describes this PR or how it makes you feel?
Summary by CodeRabbit