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

Separate Cypress tests into separate groups #927

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

Conversation

nguyenaiden
Copy link
Contributor

@nguyenaiden nguyenaiden commented Mar 26, 2022

Description: Currently, Cypress integration tests are slow because they're all running serially. The solution is to separate tests into groups - stateful and stateless, with the latter one running in parallel::

  • stateful tests (deposit, swap, withdraw)
  • stateless tests (accountDetails, advancedOptions, risk, and topMenu)

This change also requires ci.yml to be updated to accommodate the changes.
Related ticket

@@ -94,9 +94,17 @@ jobs:
# fix for forcing git to use https when pulling deps
- run: 'git config --global --replace-all url."https://github.com/".insteadOf ssh://git@github.com/'
- run: npm ci
- run: npm run cy:coverage
- name: stateful tests
Copy link
Contributor

Choose a reason for hiding this comment

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

these steps will run back to back - it may be better to define a separate job for these so they can each have their own machine. If we do this then we won't necessarily need to use the parallel flag since we're basically manually parallelizing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense. Gonna make the changes after figuring out the correct keywords. Apparently I'm using with incorrectly.

@codecov
Copy link

codecov bot commented Mar 28, 2022

Codecov Report

Merging #927 (e1f0bd0) into master (fe0d0fd) will decrease coverage by 55.53%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##           master     #927       +/-   ##
===========================================
- Coverage   67.83%   12.29%   -55.54%     
===========================================
  Files         125      126        +1     
  Lines        3830     3928       +98     
  Branches     1165     1204       +39     
===========================================
- Hits         2598      483     -2115     
- Misses       1210     3445     +2235     
+ Partials       22        0       -22     
Impacted Files Coverage Δ
src/pages/App.tsx 0.00% <ø> (-100.00%) ⬇️
src/pages/CreatePool/CreatePoolDialog.tsx 0.00% <0.00%> (ø)
src/pages/CreatePool/index.tsx 0.00% <0.00%> (ø)
src/pages/Pools.tsx 0.00% <0.00%> (-74.58%) ⬇️
src/theme/components/TextFieldTheme.ts 0.00% <ø> (-100.00%) ⬇️
src/theme/components/ToggleButtonTheme.ts 0.00% <ø> (-60.00%) ⬇️
src/i18n.ts 0.00% <0.00%> (-100.00%) ⬇️
src/index.tsx 0.00% <0.00%> (-100.00%) ⬇️
src/pages/Risk.tsx 0.00% <0.00%> (-100.00%) ⬇️
... and 104 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8da4e74...e1f0bd0. Read the comment docs.

@nguyenaiden nguyenaiden force-pushed the integration-test-group branch 3 times, most recently from 293f790 to aa18683 Compare March 29, 2022 18:43
@nguyenaiden nguyenaiden changed the title [WIP] Separate Cypress tests into groups Separate Cypress tests into separate groups Mar 29, 2022
@nguyenaiden nguyenaiden force-pushed the integration-test-group branch from ab83e07 to 7eb677d Compare March 29, 2022 18:50
@@ -5,7 +5,6 @@
"requires": true,
"packages": {
"": {
"name": "saddle-frontend",
Copy link
Contributor

Choose a reason for hiding this comment

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

👀 not really sure why this happens but please rm from PR

- 8545:8545
strategy:
fail-fast: false
matrix:
Copy link
Contributor

Choose a reason for hiding this comment

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

we can probably just remove matrix altogether if it's being run on one server

@nguyenaiden nguyenaiden force-pushed the integration-test-group branch 2 times, most recently from 5d65b1d to 96b6786 Compare March 29, 2022 19:51
Duy Nguyen added 8 commits March 30, 2022 11:38
Run both groups in parallel

Readded lost changes

Readded package del and removed integration folder
Readded repo name to package-lock.json
Update relative paths

Fixed syntax
Increased deposit wait for app to 5 secs
@nguyenaiden nguyenaiden force-pushed the integration-test-group branch from 1c9dd04 to e1f0bd0 Compare March 30, 2022 18:39
@alphastorm alphastorm force-pushed the master branch 2 times, most recently from 5a3f5c3 to 5b44a67 Compare October 20, 2023 16:21
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