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.

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

[Chore] - Updated CI and Added Caching Feature #691

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

adithyaakrishna
Copy link
Contributor

@adithyaakrishna adithyaakrishna commented Jun 16, 2023

Description:

Instead of using npm install we would be using npm ci which is usually used for CI Environments

@codecov
Copy link

codecov bot commented Jun 16, 2023

Codecov Report

Merging #691 (b533005) into master (69f3144) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #691   +/-   ##
=======================================
  Coverage   83.06%   83.07%           
=======================================
  Files         141      141           
  Lines       11456    11456           
  Branches     2114     2114           
=======================================
+ Hits         9516     9517    +1     
+ Misses       1609     1607    -2     
- Partials      331      332    +1     

see 2 files with indirect coverage changes

@adithyaakrishna adithyaakrishna marked this pull request as ready for review June 16, 2023 09:52
@adithyaakrishna
Copy link
Contributor Author

@dkoes Fixed CI and Updated CodeQL Workflow as well :)

@dkoes
Copy link
Contributor

dkoes commented Jun 16, 2023

This is still broken. You need to rerun the CI build so that you can see what happens when the cache is populated.

@adithyaakrishna adithyaakrishna marked this pull request as draft June 18, 2023 18:38
@adithyaakrishna adithyaakrishna marked this pull request as ready for review June 21, 2023 17:32
@adithyaakrishna
Copy link
Contributor Author

@dkoes I don't have permission to rerun workflow for this specific PR, but I ran it on my fork's branch and the cache works now.

Fork's Branch Action Runs

Could you please rerun the workflow for this PR?

@@ -31,8 +31,10 @@
"build:prod": "webpack --config webpack.prod.js --mode production",
"prepare": "npm run build:dev && npm run build:prod",
"postinstall": "npm run build",
"jest": "npx jest --testPathIgnorePatterns=render.test.js",
"jest": "npx jest --testPathIgnorePatterns=render.test.js --maxWorkers=50%",
"jest:ci": "npx jest --testPathIgnorePatterns=render.test.js --runInBand",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found a pretty good blog related to improving jest test speeds, https://dev.to/vantanev/make-your-jest-tests-up-to-20-faster-by-changing-a-single-setting-i36 and according to it --runInBand is useful in resource-constrained environments like CI, where the overhead of worker processes is higher than the speedup of running tests in parallel.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also benched the tests using https://github.com/sharkdp/hyperfine and these are the results that I got. Over the course of 3-4 runs, --maxWorkers=50% seemed to be faster

Screenshot 2023-06-20 at 8 46 43 PM

@adithyaakrishna adithyaakrishna requested a review from dkoes June 21, 2023 18:13
dkoes
dkoes previously approved these changes Jun 21, 2023
src/utilities.ts Fixed Show fixed Hide fixed
Adithya Krishna added 16 commits July 10, 2023 19:53
Signed-off-by: Adithya Krishna <[email protected]>
Signed-off-by: Adithya Krishna <[email protected]>
Signed-off-by: Adithya Krishna <[email protected]>
Signed-off-by: Adithya Krishna <[email protected]>
Signed-off-by: Adithya Krishna <[email protected]>
Signed-off-by: Adithya Krishna <[email protected]>
Signed-off-by: Adithya Krishna <[email protected]>
Signed-off-by: Adithya Krishna <[email protected]>
Signed-off-by: Adithya Krishna <[email protected]>
Signed-off-by: Adithya Krishna <[email protected]>
Signed-off-by: Adithya Krishna <[email protected]>
Signed-off-by: Adithya Krishna <[email protected]>
Signed-off-by: Adithya Krishna <[email protected]>
Signed-off-by: Adithya Krishna <[email protected]>
Signed-off-by: Adithya Krishna <[email protected]>
Signed-off-by: Adithya Krishna <[email protected]>
Adithya Krishna added 3 commits July 10, 2023 19:53
Signed-off-by: Adithya Krishna <[email protected]>
Signed-off-by: Adithya Krishna <[email protected]>
Signed-off-by: Adithya Krishna <[email protected]>
Signed-off-by: Adithya Krishna <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants