test: add testing groups and browser testing environment #728

Dhaiwat10 · 2023-01-13T05:49:33Z

Closes #721

Summary

This PR makes use of jest-runner-groups to divide all of our tests into four testing groups:

Node-only unit tests
- Things like CLI tools that will only run in a Node environment, anything not targeting browsers
Node-only e2e tests
- Same as above, but require fuel-core to be running in bg
Common (Node + Browser) unit tests
- Tests covering code that needs to work in both Node and Browser environments.
Common (Node + Browser) e2e tests
- Same as above, but require fuel-core to be running in bg

Workflows

I have created four new GitHub workflows, each utilizing the grouping tags to pick the relevant set of tests that need to be tested in the given environment.

Node unit tests
Node E2E tests
Browser unit tests
Browser E2E tests

To emulate a browser environment, I am using puppeteer.

Group Tagging

In this setup, we must ensure that our tests are always appropriately tagged with the relevant testing group (example).

If a new test is added but is not tagged, it will not run, and the checks for the given PR should fail. They should fail due to the resulting decrease in code-coverage percentage caused by the new source code addition for which the tests never ran.

To-do

Fix the errors, or investigate if those functions are actually not working in the browser. (Having some trouble with some native crypto libraries in the browser and one or two tests in Node too)
Coverage reporting
- Save coverage reports for individual runs
- Have a new flow that will:
  - Merge all saved reports
  - Validate them against our minimum coverage threshold
  - Publish the merged report somewhere (i.e., codecov or similar)
  - Add a coverage badge to project README, reflecting the project coverage status

Final Notes

By enforcing a minimum coverage threshold, we can disable the jest-coverage-report-action, speeding up our checks by 50%, which will be huge.

The rationale here is that this report action only serves the purpose of informing about coverage changes. In contrast, the new approach enforces a minimum threshold, guaranteeing an increased confidence level for our tests at each new PR.

Ideally, we'd have 100% coverage for everything. In the beginning, that will not be possible, but we can work it up with time while sticking to 100% regarding new additions.

This reverts commit df2e508.

github-actions · 2023-01-13T06:13:42Z

Coverage report

❌ An unexpected error occurred. For more details, check console

Error: The process '/home/runner/setup-pnpm/node_modules/.bin/pnpm' failed with exit code 1

St.^❔	Category	Percentage	Covered / Total
🟢	Statements	94.03% (-0.72% 🔻)	4332/4607
🟢	Branches	81.29% (-1.18% 🔻)	769/946
🟢	Functions	91.5% (-0.43% 🔻)	861/941
🟢	Lines	93.88% (-0.74% 🔻)	4160/4431

Show files with reduced coverage 🔻

St.^❔	File	Statements	Branches	Functions	Lines
🟢	math/src/bn.ts	95.45% (-1.14% 🔻)	87.8% (-2.44% 🔻)	96.97%	95.35% (-1.16% 🔻)
🟢	`...` / array.ts	88.24% (-11.76% 🔻)	0% (-100% 🔻)	100%	87.5% (-12.5% 🔻)
🟢	`...` / b256.ts	88.89% (-11.11% 🔻)	66.67% (-33.33% 🔻)	100%	88.89% (-11.11% 🔻)
🔴	`...` / b512.ts	50% (-50% 🔻)	0% (-100% 🔻)	66.67% (-33.33% 🔻)	50% (-50% 🔻)
🟢	`...` / boolean.ts	81.25% (-6.25% 🔻)	60% (-20% 🔻)	100%	81.25% (-6.25% 🔻)
🔴	`...` / byte.ts	21.43% (-78.57% 🔻)	0% (-100% 🔻)	0% (-100% 🔻)	21.43% (-78.57% 🔻)
🟢	`...` / enum.ts	91.67% (-8.33% 🔻)	0% (-100% 🔻)	100%	91.67% (-8.33% 🔻)
🟢	`...` / u64.ts	90.91% (-9.09% 🔻)	100%	100%	90.91% (-9.09% 🔻)
🟢	`...` / number.ts	95.24% (-4.76% 🔻)	100%	100%	95.24% (-4.76% 🔻)
🟢	`...` / struct.ts	95.83% (-4.17% 🔻)	66.67% (-33.33% 🔻)	100%	95.83% (-4.17% 🔻)
🟢	`...` / tuple.ts	94.12% (-5.88% 🔻)	0% (-100% 🔻)	100%	92.86% (-7.14% 🔻)
🟢	`...` / script-request.ts	84.44% (-1.92% 🔻)	60% (-8.75% 🔻)	88.89% (-1.11% 🔻)	84.44% (-1.92% 🔻)

Test suite run failed

Failed tests: 1/643. Failed suites: 1/99.

  ● GenericTypesContract › should call complex contract function with generic type

    expect(received).rejects.toThrow(expected)

    Expected pattern: /gasLimit\(10\) is lower than the required/
    Received message: "output already exists: {\"response\":{\"data\":null,\"errors\":[{\"message\":\"output already exists\",\"locations\":[{\"line\":2,\"column\":3}],\"path\":[\"dryRun\"]}],\"status\":200,\"headers\":{}},\"request\":{\"query\":\"mutation dryRun($encodedTransaction: HexString!, $utxoValidation: Boolean) {\\n  dryRun(tx: $encodedTransaction, utxoValidation: $utxoValidation) {\\n    ...receiptFragment\\n  }\\n}\\n\\nfragment receiptFragment on Receipt {\\n  data\\n  rawPayload\\n}\",\"variables\":{\"encodedTransaction\":\"0x00000000000000000000000000000190000000000000000a0000000000000000000000000000005000000000000000080000000000000001000000000000000100000000000000010000000000000000000000000000000000000000000000000000000000000000900000044700000000000000000000385dfcc00110fff3006148000c5d4920005d47f002104513005d43f001340004503348100024480000753820666f6f000000000000000000080000000000000038000000000000002a0000000000000000b3390aaf551091807b9f2076e08d351e2557291f3a6ec266ec472cd03f1c5503000000000000000179e49bcaf53e46e975073945e8802bd210080b8e12f88d38d093607c19b4b53900000000000013880000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000379e49bcaf53e46e975073945e8802bd210080b8e12f88d38d093607c19b4b539000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000040285d96fff1251a5d2759ade4a034e47b796e3ed2c6654d1a6eb4b086c390ac013717b4627da87bb4b96d5a8d6963fd4d02f5e71c0b43219f90c23e2a5a57d66e\",\"utxoValidation\":false}}}"

      76 |     expect(async () => {
      77 |       await scriptInstance.functions.main(foo).txParams({ gasLimit: 10, gasPrice: 400 }).call();
    > 78 |     }).rejects.toThrow(/gasLimit\(10\) is lower than the required/);
         |                ^
      79 |   });
      80 | });
      81 |

      at node_modules/.pnpm/[email protected][email protected]/node_modules/graphql-request/src/index.ts:497:11
      at step (node_modules/.pnpm/[email protected][email protected]/node_modules/graphql-request/dist/index.js:63:23)
      at Object.next (node_modules/.pnpm/[email protected][email protected]/node_modules/graphql-request/dist/index.js:44:53)
      at fulfilled (node_modules/.pnpm/[email protected][email protected]/node_modules/graphql-request/dist/index.js:35:58)
      at Object.toThrow (node_modules/.pnpm/[email protected]/node_modules/expect/build/index.js:210:22)
      at Object.<anonymous> (packages/fuel-gauge/src/script-main-args.node.e2e.test.ts:78:16)

Report generated by 🧪jest coverage report action from 5e7427c

Dhaiwat10 · 2023-02-27T14:00:32Z

@arboleya I think we definitely can. Let me get on it

Dhaiwat10 · 2023-02-27T20:01:21Z

@arboleya I've made it so that the newly added workflows will only run for this PR's branch. I think we can safely merge this to master if everything else looks good, and then open another PR to tackle coverage reporting.

camsjams

Had some notes

camsjams · 2023-03-01T06:58:54Z

.github/workflows/browser-e2e-tests.yaml

+      - name: CI Setup
+        uses: ./.github/actions/ci-setup
+
+      - name: Build


These steps are repeated and should all be abstracted away into composite actions, such as a wrapper like this:
https://github.com/FuelLabs/fuels-ts/blob/master/.github/actions/ci-setup/action.yaml

That would apply to all of these "setup" steps as well as the "coverage" and "teardown" steps after.

Also have you looked at using the advanced GitHub action called a test matrix?

Alongside the test matrix idea, you could also investigate fail-fast or the similar continue-on-error to let builds find the bad linting first, then bad unit tests, then get into the meaty stuff like e2e

camsjams · 2023-03-01T07:01:19Z

.github/workflows/node-e2e-tests.yaml

+jobs:
+  node-e2e-tests:
+    # only run this job if the base branch is dp/testing-groups
+    if: ${{github.ref_name == '728/merge'}}


You can remove this check from all the files as this action only exists on your PR anyways, unless you had noticed some kind of weird side effect

We are having to keep this here because we will need to merge this PR in before being fully done with this whole setup, as things stand. Please see: #728 (comment)

camsjams · 2023-03-01T07:03:23Z

.github/workflows/test-coverage.yaml

+          path: node-e2e-coverage
+
+      - name: Merge all coverage reports
+        run: npx istanbul-merge --out coverage-merged.json browser-unit-coverage/coverage-final.json browser-e2e-coverage/coverage-final.json node-unit-coverage/coverage-final.json node-e2e-coverage/coverage-final.json


camsjams · 2023-03-01T07:05:42Z

packages/abi-coder/src/abi-coder.test.ts

@@ -6,6 +6,9 @@ import type { DecodedValue } from './coders/abstract-coder';

 const B256 = '0xd5579c46dfcc7f18207013e65b44e4cb4e2c2298f4ac457ba8f82743f31e930b';

+/*
+ * @group node/unit
+ */


What happens if someone forgets to mark the group in the file?

Usually when I split tests out into different groups I just change the file extension:
abi-coder.unit.test.ts
and
contract-usage.e2e.test.ts
For us maybe even:
contract-usage.node.e2e.test.ts

Easy to glob:
*.unit.test.ts finds all unit tests
*.e2e.test.ts finds all e2e tests
*.node.e2e.test.ts finds all node e2e tests (feel free to make this shorter somehow)
*.test.ts finds all tests

Its also easier to see in the file tree on GitHub/shell/IDE what kind of test a given file is without hunting through the file source for the group name.

If someone forgets to mark the group, those tests won't run. I agree that this is not ideal and that's why I have switched to the approach you suggested instead. Things are running fine locally, but for some reason when I'm trying to run the tests in Github actions it's not being able to pattern match 🤔:

Run npx jest -- "**/*.node.unit.test.ts" --coverage Invalid testPattern **/*.node.unit.test.ts|--coverage supplied. Running all tests instead.

Link to the logs

camsjams · 2023-03-01T07:07:20Z

package.json

@@ -70,12 +70,14 @@
    "forc-bin": "workspace:*",
    "husky": "^8.0.3",
    "jest": "^29.3.1",
+    "jest-runner-groups": "^2.2.0",


I think its easier to change file names instead of adding a dependency to rely on that could cause issue

camsjams · 2023-03-01T07:18:42Z

Also don't forget post merge that the repo settings on GH has to be updated to change the required checks list:

camsjams · 2023-03-01T07:22:27Z

.github/workflows/test.yaml

      - name: Lint
        run: |
          pnpm lint
-
      - name: Checking PR Number


Should the Run tests and publish coverage step below be removed?

When we merge this PR, we still want the rest of the repo to use this old test action instead of the new one. More context here: #728 (comment)

That's why I have kept it intact still.

camsjams · 2023-03-20T05:35:37Z

.github/workflows/browser-unit-tests.yaml

+
+      - name: Build
+        run: |
+          BUILD_VERSION="0.0.0-${{ github.ref_name }}-$(git rev-parse --short $GITHUB_SHA)" pnpm build


as mentioned, still need to at very least move these into an action

camsjams · 2023-03-20T05:36:21Z

.github/workflows/browser-unit-tests.yaml

+          BUILD_VERSION="0.0.0-${{ github.ref_name }}-$(git rev-parse --short $GITHUB_SHA)" pnpm build
+
+      - name: Browser unit tests
+        run: npx jest -- "**/*.common.unit.test.ts" --setupFilesAfterEnv=./setup-puppeteer.ts --testTimeout=30000 --coverage


This just launches puppeteer and then runs the tests on node?

arboleya · 2023-04-05T16:32:33Z

@Dhaiwat10 Is it worth resolving conflicts here?

Or would extracting the good parts/workflows into a new PR be easier?

Dhaiwat10 · 2023-04-05T16:39:38Z

@arboleya not worth resolving the conflicts. I was planning on opening a new PR once the build tooling revamp was done since the Karma-Jasmine setup that we tried was having trouble reading the .ts files (it was expecting built-out .js files).

Dhaiwat10 added 3 commits January 13, 2023 10:34

add jest-runner-groups and tag tests

9ee8d19

add jsdom and new workflow file to test

28fbf75

fix typo in workflow file

df2e508

Dhaiwat10 changed the title ~~test: add testing groups~~ test: add testing groups and browser testing environment Jan 13, 2023

Dhaiwat10 added 2 commits January 13, 2023 11:29

Revert "fix typo in workflow file"

5ccf2b6

This reverts commit df2e508.

fix usage of pnpm test in CI

8799f16

Dhaiwat10 added 6 commits January 13, 2023 15:10

run all 4 test groups in parallel

4abf41b

rename workflow files

b647cca

fix some misgrouped tests

5aa382a

run services for e2e tests

8e50548

add empty changeset

63d54a8

polyfill TextEncoder and TextDecoder

51d4bb4

Dhaiwat10 mentioned this pull request Jan 16, 2023

test: add test for browser compatibility #721

Closed

arboleya assigned Dhaiwat10 Jan 17, 2023

Merge branch 'master' into dp/testing-groups

f4e8481

arboleya self-assigned this Jan 17, 2023

Dhaiwat10 added 7 commits January 31, 2023 13:39

Merge branch 'master' into dp/testing-groups

b790b7b

use puppeteer for browser unit tests

4bc2268

use pupeteer for all browser tests, remove jsdom

35faf15

delete old test workflow, move lint checks

7677df5

add build step to lint action

79b3bb0

run e2e tests sequentially

1aa45ce

Merge branch 'master' into dp/testing-groups

df94fce

arboleya mentioned this pull request Feb 6, 2023

Add Buffer compatibility for browsers #765

Closed

Dhaiwat10 added 5 commits February 25, 2023 00:26

Merge branch 'master' into dp/testing-groups

9127f1c

add label to account.test.ts

a49de06

try merging coverage reports using gh artifacts

c3338b6

temp remove node e2e tests from reqd list bc theyre failing

05d748b

Update merge-coverage.yaml

75665c1

Dhaiwat10 added 9 commits February 27, 2023 21:14

run workflows only for this pr

e96513b

only run workflow for this pr

3549d51

only run workflow for this pr

ab2f3b6

only run workflow for this pr

3bc6561

only run workflow for this pr

9058236

Update browser-e2e-tests.yaml

131fbcd

fix

7a3e7ff

reset pr-lint workflow

8b460f4

Merge branch 'master' into dp/testing-groups

25f416c

Dhaiwat10 marked this pull request as ready for review February 27, 2023 20:00

camsjams reviewed Mar 1, 2023

View reviewed changes

Dhaiwat10 added 7 commits March 6, 2023 20:41

remove jest-runner-groups

bd5354e

rename test file

863f636

remove --group

4f32fa5

change patterns

1a79736

try npx jest instead of pnpm test

0b1086a

update patterns

7ac4759

Update sumMerkleTree.common.unit.test.ts

5e7427c

Dhaiwat10 mentioned this pull request Mar 13, 2023

Move from Jest to a more suitable alternative #813

Closed

camsjams suggested changes Mar 20, 2023

View reviewed changes

Dhaiwat10 closed this Apr 17, 2023

arboleya mentioned this pull request Jun 5, 2023

Introduce a TS SDK testing standard and approach #1043

Closed

4 tasks

danielbate mentioned this pull request Aug 17, 2023

feat: implement playwright for browser compatibility testing #1182

Closed

Dhaiwat10 deleted the dp/testing-groups branch February 23, 2024 23:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: add testing groups and browser testing environment #728

test: add testing groups and browser testing environment #728

Dhaiwat10 commented Jan 13, 2023 •

edited

Loading

github-actions bot commented Jan 13, 2023 •

edited

Loading

Dhaiwat10 commented Feb 27, 2023

Dhaiwat10 commented Feb 27, 2023 •

edited

Loading

camsjams left a comment

camsjams Mar 1, 2023

camsjams Mar 1, 2023

camsjams Mar 1, 2023 •

edited

Loading

Dhaiwat10 Mar 6, 2023

camsjams Mar 1, 2023

camsjams Mar 1, 2023 •

edited

Loading

Dhaiwat10 Mar 6, 2023 •

edited

Loading

camsjams Mar 1, 2023

camsjams commented Mar 1, 2023

camsjams Mar 1, 2023

Dhaiwat10 Mar 6, 2023

camsjams Mar 20, 2023

camsjams Mar 20, 2023

arboleya commented Apr 5, 2023

Dhaiwat10 commented Apr 5, 2023

test: add testing groups and browser testing environment #728

test: add testing groups and browser testing environment #728

Conversation

Dhaiwat10 commented Jan 13, 2023 • edited Loading

Summary

Workflows

Group Tagging

To-do

Final Notes

github-actions bot commented Jan 13, 2023 • edited Loading

Coverage report

Test suite run failed

Dhaiwat10 commented Feb 27, 2023

Dhaiwat10 commented Feb 27, 2023 • edited Loading

camsjams left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

camsjams Mar 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

camsjams Mar 1, 2023 • edited Loading

Choose a reason for hiding this comment

Dhaiwat10 Mar 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

camsjams commented Mar 1, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arboleya commented Apr 5, 2023

Dhaiwat10 commented Apr 5, 2023

Dhaiwat10 commented Jan 13, 2023 •

edited

Loading

github-actions bot commented Jan 13, 2023 •

edited

Loading

Dhaiwat10 commented Feb 27, 2023 •

edited

Loading

camsjams Mar 1, 2023 •

edited

Loading

camsjams Mar 1, 2023 •

edited

Loading

Dhaiwat10 Mar 6, 2023 •

edited

Loading