Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Adding Lighthouse audit #445

Merged
merged 74 commits into from
May 1, 2021
Merged

Adding Lighthouse audit #445

merged 74 commits into from
May 1, 2021

Conversation

RichtXO
Copy link
Member

@RichtXO RichtXO commented Apr 14, 2021

Was just thinking of automating this audit for front-end changes

Additional Info
We should have had this a long time ago :/

The minScore should be changed before merging with the master. The current values are in place for the current website!

@RichtXO
Adding Lighthouse audit
5b15f28 
Was just thinking of automating this process
@RichtXO RichtXO marked this pull request as draft April 15, 2021 00:02
RichtXO added 2 commits April 14, 2021 20:13
@RichtXO
made changes
4ab7df9 
what's docker again?
@RichtXO
run plz?
05d3679
@123joshuawu
Copy link
Member

can add this as a job to the CI workflow

and not sure if we will need data for this to work properly, but if so, can use this e2e workflow as reference https://github.com/YACS-RCOS/yacs.n/pull/251/files

@YACS-RCOS YACS-RCOS deleted a comment from github-actions bot Apr 15, 2021
@YACS-RCOS YACS-RCOS deleted a comment from github-actions bot Apr 15, 2021
@YACS-RCOS YACS-RCOS deleted a comment from github-actions bot Apr 15, 2021
@YACS-RCOS YACS-RCOS deleted a comment from github-actions bot Apr 15, 2021
@github-actions
Copy link

Deploy Link: https://172.104.215.175/
Branch and Build Info: https://172.104.215.175:3000/info.txt
Expected Time Ready: Thu Apr 15 00:27:57 EDT 2021

@github-actions
Copy link

Deploy Link: https://45.79.149.39/
Branch and Build Info: https://45.79.149.39:3000/info.txt
Expected Time Ready: Thu Apr 15 00:30:41 EDT 2021

@github-actions
Copy link

Deploy Link: https://97.107.142.245/
Branch and Build Info: https://97.107.142.245:3000/info.txt
Expected Time Ready: Thu Apr 15 01:26:57 EDT 2021

123joshuawu
123joshuawu reviewed Apr 17, 2021
View reviewed changes
.github/workflows/pr.yaml Outdated
AUTH_ARGS: ${{ secrets.AUTH_ARGS }}
API_ENDPOINT: https://localhost/api/bulkCourseUpload

- name: run Lighthouse CI
Copy link
Member

Choose a reason for hiding this comment

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

could also explore using a lighthouse github action like https://github.com/marketplace/actions/lighthouse-check or https://github.com/marketplace/actions/lighthouse-ci-action

Copy link
Member Author

Choose a reason for hiding this comment

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

Will definitely take a look at that

Copy link
Member Author

Choose a reason for hiding this comment

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

Looked into them, and they just take info from the lighthouserc.js and integrate them into the yaml file. I think it would be better to specify the specs and other lighthouse-related things in a separate file.

@RichtXO
Copy link
Member Author

RichtXO commented Apr 17, 2021

This is awesome!!

Finding this in the logs, do we want to fail the test if below the threshold? or do we want to always succeed?

https://github.com/YACS-RCOS/yacs.n/runs/2358958949#step:6:33

 ⚠️  categories.performance warning for minScore assertion
        expected: >=0.8
           found: 0.11
      all values: 0.08, 0.11, 0.1

We can definitely change warn to error, and the values for minScore. But it seems ever since PWA was introduced, it's not looking good lol. I guess it's best to set a concrete value and merge as admin. I only let that pass just to have green checks.

@RichtXO RichtXO removed the Review Ready! Inform team that PR is ready for review label Apr 20, 2021
@github-actions
Copy link

Deploy Link: https://198.74.59.213/
Branch and Build Info: https://198.74.59.213:3000/info.txt
Expected Time Ready: Tue Apr 20 01:17:14 EDT 2021

@github-actions
Copy link

Deploy Link: https://45.79.154.198/
Branch and Build Info: https://45.79.154.198:3000/info.txt
Expected Time Ready: Fri Apr 23 16:24:37 EDT 2021

RichtXO added 14 commits April 23, 2021 16:43
@RichtXO
trying out terosh/lighthouse-ci-action
dbbdbf3
@RichtXO
removing spaces
e67bb7e
@RichtXO
changing back
0998119 
Had to move the lighthouserc.js to the workflow folder for it to work, and dont want to do that.
Also, using the treosh action just takes some thing out of the js file anyway, and having all lighthouse config stuff in 1 file is better to manage
@RichtXO
adding url
b7d044f
@RichtXO
adding url
95b85ab
@RichtXO
Revert "name changed"
0a33b1b 
This reverts commit 30a9086.
@RichtXO
asdf
e08c0f4
@RichtXO
Revert "asdf"
e728192 
This reverts commit e08c0f4.
@RichtXO
Revert "Revert "name changed""
839b53c 
This reverts commit 0a33b1b.
@RichtXO
Revert "adding url"
fe5403c 
This reverts commit 95b85ab.
@RichtXO
Revert "adding url"
b124e3a 
This reverts commit b7d044f.
@RichtXO
Revert "changing back"
a58306f 
This reverts commit 0998119.
@RichtXO
Revert "removing spaces"
a9d53a3 
This reverts commit e67bb7e.
@RichtXO
Revert "trying out terosh/lighthouse-ci-action"
95df95f 
This reverts commit dbbdbf3.
@github-actions
Copy link

Deploy Link: https://66.175.212.223/
Branch and Build Info: https://66.175.212.223:3000/info.txt
Expected Time Ready: Fri Apr 23 17:42:51 EDT 2021

@RichtXO RichtXO added the Review Ready! Inform team that PR is ready for review label Apr 23, 2021
@RichtXO RichtXO merged commit 8056493 into master May 1, 2021
@RichtXO RichtXO deleted the lighthouseCI branch May 1, 2021 05:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jshom jshom jshom approved these changes

@123joshuawu 123joshuawu 123joshuawu approved these changes

Assignees
No one assigned
Labels
Review Ready! Inform team that PR is ready for review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@RichtXO @123joshuawu @jshom @Viyerelu23333