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: cleanup mocha/chai/sinon usage across the repo #6311

Merged
merged 24 commits into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 0 additions & 21 deletions .c8rc.json

This file was deleted.

35 changes: 17 additions & 18 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ module.exports = {
browser: true,
es6: true,
node: true,
// Performance tests still use mocha
mocha: true,
},
globals: {
Expand Down Expand Up @@ -110,7 +111,11 @@ module.exports = {
"error",
{
groups: ["builtin", "external", "internal", "parent", "sibling", "index"],
pathGroups: [{pattern: "@lodestar/**", group: "internal"}],
pathGroups: [
{pattern: "@lodestar/**", group: "internal"},
// We want mocks to be imported before any internal code
{pattern: "**/mocks/**", group: "internal"},
],
pathGroupsExcludedImportTypes: ["builtin"],
},
],
Expand Down Expand Up @@ -201,24 +206,18 @@ module.exports = {
},
{
files: ["**/test/**/*.test.ts"],
plugins: ["mocha", "chai-expect"],
extends: ["plugin:mocha/recommended", "plugin:chai-expect/recommended"],
plugins: ["vitest"],
extends: ["plugin:vitest/recommended"],
rules: {
// We observed that having multiple top level "describe" save valuable indentation
// https://github.com/lo1tuma/eslint-plugin-mocha/blob/master/docs/rules/max-top-level-suites.md
"mocha/max-top-level-suites": "off",
// We need to disable because we disabled "mocha/no-setup-in-describe" rule
// TODO: Move all setup code to before/beforeEach and then disable async describe
// https://github.com/lo1tuma/eslint-plugin-mocha/blob/master/docs/rules/no-async-describe.md
"mocha/no-async-describe": "off",
// Use of arrow functions are very common
"mocha/no-mocha-arrows": "off",
// It's common to call function inside describe block
// https://github.com/lo1tuma/eslint-plugin-mocha/blob/master/docs/rules/no-setup-in-describe.md
"mocha/no-setup-in-describe": "off",
// We use to split before in small isolated tasks
// https://github.com/lo1tuma/eslint-plugin-mocha/blob/master/docs/rules/no-sibling-hooks.md
"mocha/no-sibling-hooks": "off",
"vitest/consistent-test-it": ["error", {fn: "it", withinDescribe: "it"}],
// We use a lot dynamic assertions so tests may not have usage of expect
"vitest/expect-expect": "off",
"vitest/no-disabled-tests": "error",
matthewkeil marked this conversation as resolved.
Show resolved Hide resolved
"vitest/no-focused-tests": "error",
"vitest/prefer-called-with": "error",
"vitest/prefer-spy-on": "error",
// Our usage contains dynamic test title, this rule enforce static string value
"vitest/valid-title": "off",
},
},
{
Expand Down
4 changes: 0 additions & 4 deletions .mocharc.yaml

This file was deleted.

17 changes: 0 additions & 17 deletions .nycrc.json

This file was deleted.

3 changes: 3 additions & 0 deletions .wordlist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ UPnP
UTF
VM
Vitalik
Vitest
Wagyu
api
async
Expand Down Expand Up @@ -174,6 +175,7 @@ scalability
secp
sepolia
sharding
src
ssz
stakers
subnet
Expand All @@ -188,6 +190,7 @@ util
utils
validator
validators
vitest
wip
xcode
yaml
Expand Down
18 changes: 14 additions & 4 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,25 @@ Contributing to tests:
- Do not pull unpinned versions from DockerHub (use deterministic tag) or Github (checkout commit not branch).
- Carefully design tests that depend on timing sensitive events like p2p network e2e tests. Consider that Github runners are significantly less powerful than your development environment.

### Common Issues

**Error: [vitest] Cannot mock "../../src/db/repositories/index.js" because it is already loaded by "src/db/beacon.ts"**

If you observe any error in tests with matching to above error message, that implies you are loading the mocks in the wrong order. The correct order is to import the mocks first and then the actual module. We suggest to import the mocks on very top before any local modules.

**✖ Error: Cannot find package 'async_hooks' imported from**

If you observe following error running any of the test files that means you are running a file which itself or any dependency of that file imports `vitest`, but you are not running that file with `vitest` runner. Try running it with `yarn vitest` command, not with `node` command.

### Debugging Spec Tests

- To fix errors always focus on passing all minimal tests first without running mainnet tests.
- Spec tests often compare full expected vs actual states in JSON format. To better understand the diff it's convenient to use mocha's option `--inline-diffs`.
- A single logical error can cause many spec tests to fail. To focus on a single test at a time you can use mocha's option `--bail` to stop at the first failed test
- To then run only that failed test you can run against a specific file as use mocha's option `--grep` to run only one case
- Spec tests often compare full expected vs actual states in JSON format.
- A single logical error can cause many spec tests to fail. To focus on a single test at a time you can use vitest's option `--bail` to stop at the first failed test
- To then run only that failed test you can run against a specific file as use vitest's filters to run only one case

```sh
LODESTAR_PRESET=minimal ../../node_modules/.bin/mocha --config .mocharc.spec.yml test/spec/phase0/sanity.test.ts --inline-diffs --bail --grep "attestation"
LODESTAR_PRESET=minimal ../../node_modules/.bin/vitest --config vitest.config.spec.ts --run test/spec/phase0/sanity.test.ts
matthewkeil marked this conversation as resolved.
Show resolved Hide resolved
nazarhussain marked this conversation as resolved.
Show resolved Hide resolved
```

## Docker
Expand Down
2 changes: 1 addition & 1 deletion docs/pages/contribution/testing/integration-tests.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ There are two ENV variables that are required to run this test:

The command to run this test is:

`EL_BINARY_DIR=g11tech/geth:withdrawals EL_SCRIPT_DIR=gethdocker yarn mocha test/sim/withdrawal-interop.test.ts`
`EL_BINARY_DIR=g11tech/geth:withdrawals EL_SCRIPT_DIR=gethdocker yarn vitest --run test/sim/withdrawal-interop.test.ts`

The images used by this test during CI are:

Expand Down
12 changes: 0 additions & 12 deletions karma.base.config.js
matthewkeil marked this conversation as resolved.
Outdated
Show resolved Hide resolved

This file was deleted.

40 changes: 8 additions & 32 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,69 +47,45 @@
"devDependencies": {
"@chainsafe/eslint-plugin-node": "^11.2.3",
"@dapplion/benchmark": "^0.2.4",
"@types/chai": "^4.3.6",
"@types/chai-as-promised": "^7.1.6",
"@types/mocha": "^10.0.1",
"@types/mocha": "^10.0.6",
matthewkeil marked this conversation as resolved.
Show resolved Hide resolved
"@types/node": "^20.6.5",
"@types/sinon": "^10.0.16",
"@types/sinon-chai": "^3.2.9",
"@typescript-eslint/eslint-plugin": "6.7.2",
"@typescript-eslint/parser": "6.7.2",
"@vitest/coverage-v8": "^1.1.0",
"@vitest/browser": "^1.1.0",
"c8": "^8.0.1",
"chai": "^4.3.8",
"chai-as-promised": "^7.1.1",
"@vitest/coverage-v8": "^1.2.1",
"@vitest/browser": "^1.2.1",
"codecov": "^3.8.3",
"crypto-browserify": "^3.12.0",
"electron": "^26.2.2",
"eslint": "^8.50.0",
"eslint-import-resolver-typescript": "^3.6.1",
"eslint-plugin-chai-expect": "^3.0.0",
"eslint-plugin-import": "^2.28.1",
"eslint-plugin-mocha": "^10.2.0",
"eslint-plugin-prettier": "^5.0.0",
"eslint-plugin-vitest": "^0.3.20",
"https-browserify": "^1.0.0",
"jsdom": "^23.0.1",
"karma": "^6.4.2",
"karma-chai": "^0.1.0",
"karma-chrome-launcher": "^3.2.0",
"karma-cli": "^2.0.0",
"karma-electron": "^7.3.0",
"karma-firefox-launcher": "^2.1.2",
"karma-mocha": "^2.0.1",
"karma-spec-reporter": "^0.0.36",
"karma-webpack": "^5.0.0",
"lerna": "^7.3.0",
"libp2p": "1.1.1",
"mocha": "^10.2.0",
"node-gyp": "^9.4.0",
"npm-run-all": "^4.1.5",
"nyc": "^15.1.0",
"path-browserify": "^1.0.1",
"prettier": "^3.0.3",
"process": "^0.11.10",
"resolve-typescript-plugin": "^2.0.1",
"sinon": "^16.0.0",
"sinon-chai": "^3.7.0",
"stream-browserify": "^3.0.0",
"stream-http": "^3.2.0",
"supertest": "^6.3.3",
"ts-loader": "^9.4.4",
"ts-node": "^10.9.1",
"typescript": "^5.2.2",
"typescript-docs-verifier": "^2.5.0",
"vite-plugin-node-polyfills": "^0.18.0",
"vite-plugin-node-polyfills": "^0.19.0",
"vite-plugin-top-level-await": "^1.4.1",
"vitest": "^1.1.0",
"vitest-when": "^0.3.0",
"vitest": "^1.2.1",
"vitest-when": "^0.3.1",
"wait-port": "^1.1.0",
"webdriverio": "^8.27.0",
"webpack": "^5.88.2"
"webdriverio": "^8.28.0"
},
"resolutions": {
"dns-over-http-resolver": "^2.1.1",
"chai": "^4.3.10",
"loupe": "^2.3.6",
"vite": "^5.0.0"
}
Expand Down
2 changes: 1 addition & 1 deletion packages/api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
"lint:fix": "yarn run lint --fix",
"pretest": "yarn run check-types",
"test": "yarn test:unit && yarn test:e2e",
"test:unit": "vitest --run --dir test/unit/ --coverage",
matthewkeil marked this conversation as resolved.
Show resolved Hide resolved
"test:unit": "vitest --run --dir test/unit/",
"check-readme": "typescript-docs-verifier"
},
"dependencies": {
Expand Down
1 change: 1 addition & 0 deletions packages/api/test/perf/compileRouteUrlFormater.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {compileRouteUrlFormater} from "../../src/utils/urlFormat.js";
/* eslint-disable no-console */

describe("route parse", () => {
// eslint-disable-next-line vitest/no-disabled-tests
nazarhussain marked this conversation as resolved.
Show resolved Hide resolved
it.skip("Benchmark compileRouteUrlFormater", () => {
const path = "/eth/v1/validator/:name/attester/:epoch";
const args = {epoch: 5, name: "HEAD"};
Expand Down
1 change: 0 additions & 1 deletion packages/api/test/utils/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ export function getTestServer(): {baseUrl: string; server: FastifyInstance} {
return {baseUrl, server};
}

/** Type helper to get a Sinon mock object type with Api */
export function getMockApi<Api extends Record<string, any>>(
routeIds: Record<string, any>
): MockedObject<ServerApi<Api>> & ServerApi<Api> {
Expand Down
2 changes: 1 addition & 1 deletion packages/api/vitest.config.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {defineConfig, mergeConfig} from "vitest/config";
import vitestConfig from "../../vitest.base.config";
import vitestConfig from "../../vitest.base.unit.config";

export default mergeConfig(
vitestConfig,
Expand Down
16 changes: 8 additions & 8 deletions packages/beacon-node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,15 @@
"lint:fix": "yarn run lint --fix",
"pretest": "yarn run check-types",
"test": "yarn test:unit && yarn test:e2e",
"test:unit:minimal": "vitest --run --segfaultRetry 3 --dir test/unit/ --coverage",
"test:unit:mainnet": "LODESTAR_PRESET=mainnet nyc --cache-dir .nyc_output/.cache -e .ts mocha 'test/unit-mainnet/**/*.test.ts'",
"test:unit:minimal": "vitest --run --segfaultRetry 3 --dir test/unit/",
"test:unit:mainnet": "LODESTAR_PRESET=mainnet vitest --run --dir test/unit-mainnet",
"test:unit": "wrapper() { yarn test:unit:minimal $@ && yarn test:unit:mainnet $@; }; wrapper",
"test:e2e": "LODESTAR_PRESET=minimal vitest --run --segfaultRetry 3 --poolOptions.threads.singleThread true --dir test/e2e",
"test:sim": "mocha 'test/sim/**/*.test.ts'",
"test:sim:merge-interop": "mocha 'test/sim/merge-interop.test.ts'",
"test:sim:mergemock": "mocha 'test/sim/mergemock.test.ts'",
"test:sim:withdrawals": "mocha 'test/sim/withdrawal-interop.test.ts'",
"test:sim:blobs": "mocha 'test/sim/4844-interop.test.ts'",
"test:e2e": "LODESTAR_PRESET=minimal vitest --run --segfaultRetry 3 --config vitest.config.e2e.ts --dir test/e2e",
"test:sim": "vitest --run test/sim/**/*.test.ts",
"test:sim:merge-interop": "vitest --run test/sim/merge-interop.test.ts",
"test:sim:mergemock": "vitest --run test/sim/mergemock.test.ts",
"test:sim:withdrawals": "vitest --run test/sim/withdrawal-interop.test.ts",
"test:sim:blobs": "vitest --run test/sim/4844-interop.test.ts",
"download-spec-tests": "node --loader=ts-node/esm test/spec/downloadTests.ts",
"test:spec:bls": "vitest --run --config vitest.config.spec.ts --dir test/spec/bls/",
"test:spec:general": "vitest --run --config vitest.config.spec.ts --dir test/spec/general/",
Expand Down
44 changes: 0 additions & 44 deletions packages/beacon-node/test/__mocks__/apiMocks.ts

This file was deleted.

Loading
Loading