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

test: migrate api unit tests to vitest #6177

Merged
merged 3 commits into from
Dec 11, 2023
Merged
Show file tree
Hide file tree
Changes from 2 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
8 changes: 0 additions & 8 deletions packages/api/.mocharc.yaml

This file was deleted.

3 changes: 0 additions & 3 deletions packages/api/.nycrc.json

This file was deleted.

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": "nyc --cache-dir .nyc_output/.cache -e .ts mocha 'test/unit/**/*.test.ts'",
"test:unit": "vitest --run --dir test/unit/ --coverage",
"check-readme": "typescript-docs-verifier"
},
"dependencies": {
Expand Down
2 changes: 2 additions & 0 deletions packages/api/test/globalSetup.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export async function setup(): Promise<void> {}
export async function teardown(): Promise<void> {}
6 changes: 0 additions & 6 deletions packages/api/test/setup.ts

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {describe} from "vitest";
import {createChainForkConfig, defaultChainConfig} from "@lodestar/config";
import {Api, ReqTypes} from "../../../../src/beacon/routes/beacon/index.js";
import {getClient} from "../../../../src/beacon/client/beacon.js";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {expect} from "chai";
import {describe, it, expect} from "vitest";
import {config} from "@lodestar/config/default";
import {Api, ReqTypes, getReturnTypes} from "../../../../src/beacon/routes/config.js";
import {getClient} from "../../../../src/beacon/client/config.js";
Expand Down Expand Up @@ -27,6 +27,6 @@ describe("beacon / config", () => {
const jsonRes = returnTypes.getSpec.toJson({data: partialJsonSpec});
const specRes = returnTypes.getSpec.fromJson(jsonRes);

expect(specRes).to.deep.equal({data: partialJsonSpec}, "Wrong toJson -> fromJson");
expect(specRes).toEqual({data: partialJsonSpec});
});
});
76 changes: 39 additions & 37 deletions packages/api/test/unit/beacon/genericServerTest/debug.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {expect} from "chai";
import {describe, it, expect, MockInstance} from "vitest";
import {toHexString} from "@chainsafe/ssz";
import {ssz} from "@lodestar/types";
import {config} from "@lodestar/config/default";
Expand All @@ -11,40 +11,42 @@ import {registerRoute} from "../../../../src/utils/server/registerRoute.js";
import {HttpClient} from "../../../../src/utils/client/httpClient.js";
import {testData} from "../testData/debug.js";

describe("beacon / debug", function () {
describe(
"beacon / debug",
function () {
describe("Run generic server test", () => {
runGenericServerTest<Api, ReqTypes>(config, getClient, getRoutes, testData);
});

// Get state by SSZ

describe("getState() in SSZ format", () => {
const {baseUrl, server} = getTestServer();
const mockApi = getMockApi<Api>(routesData);
for (const route of Object.values(getRoutes(config, mockApi))) {
registerRoute(server, route);
}

for (const method of ["getState" as const, "getStateV2" as const]) {
it(method, async () => {
const state = ssz.phase0.BeaconState.defaultValue();
const stateSerialized = ssz.phase0.BeaconState.serialize(state);
(mockApi[method] as MockInstance).mockResolvedValue(stateSerialized);

const httpClient = new HttpClient({baseUrl});
const client = getClient(config, httpClient);

const res = await client[method]("head", "ssz");

expect(res.ok).toBe(true);

if (res.ok) {
expect(toHexString(res.response)).toBe(toHexString(stateSerialized));
}
});
}
});
},
// Extend timeout since states are very big
this.timeout(30 * 1000);

describe("Run generic server test", () => {
runGenericServerTest<Api, ReqTypes>(config, getClient, getRoutes, testData);
});

// Get state by SSZ

describe("getState() in SSZ format", () => {
const {baseUrl, server} = getTestServer();
const mockApi = getMockApi<Api>(routesData);
for (const route of Object.values(getRoutes(config, mockApi))) {
registerRoute(server, route);
}

for (const method of ["getState" as const, "getStateV2" as const]) {
it(method, async () => {
const state = ssz.phase0.BeaconState.defaultValue();
const stateSerialized = ssz.phase0.BeaconState.serialize(state);
mockApi[method].resolves(stateSerialized);

const httpClient = new HttpClient({baseUrl});
const client = getClient(config, httpClient);

const res = await client[method]("head", "ssz");

expect(res.ok).to.be.true;

if (res.ok) {
expect(toHexString(res.response)).to.equal(toHexString(stateSerialized), "returned state value is not equal");
}
});
}
});
});
{timeout: 30 * 1000}
);
12 changes: 7 additions & 5 deletions packages/api/test/unit/beacon/genericServerTest/events.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {expect} from "chai";
import {describe, it, expect, beforeEach, afterEach} from "vitest";
import {sleep} from "@lodestar/utils";
import {config} from "@lodestar/config/default";
import {Api, routesData, EventType, BeaconEvent} from "../../../../src/beacon/routes/events.js";
Expand All @@ -16,7 +16,9 @@ describe("beacon / events", () => {
}

let controller: AbortController;
beforeEach(() => (controller = new AbortController()));
beforeEach(() => {
controller = new AbortController();
});
afterEach(() => controller.abort());

it("Receive events", async () => {
Expand All @@ -38,9 +40,9 @@ describe("beacon / events", () => {
const eventsReceived: BeaconEvent[] = [];

await new Promise<void>((resolve, reject) => {
mockApi.eventstream.callsFake(async (topics, signal, onEvent) => {
mockApi.eventstream.mockImplementation(async (topics, signal, onEvent) => {
try {
expect(topics).to.deep.equal(topicsToRequest, "Wrong received topics");
expect(topics).toEqual(topicsToRequest);
for (const event of eventsToSend) {
onEvent(event);
await sleep(5);
Expand All @@ -58,6 +60,6 @@ describe("beacon / events", () => {
});
});

expect(eventsReceived).to.deep.equal(eventsToSend, "Wrong received events");
expect(eventsReceived).toEqual(eventsToSend);
});
});
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {describe} from "vitest";
import {config} from "@lodestar/config/default";
import {Api, ReqTypes} from "../../../../src/beacon/routes/lightclient.js";
import {getClient} from "../../../../src/beacon/client/lightclient.js";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {describe} from "vitest";
import {config} from "@lodestar/config/default";
import {Api, ReqTypes} from "../../../../src/beacon/routes/node.js";
import {getClient} from "../../../../src/beacon/client/node.js";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {describe} from "vitest";
import {config} from "@lodestar/config/default";
import {Api, ReqTypes} from "../../../../src/beacon/routes/proof.js";
import {getClient} from "../../../../src/beacon/client/proof.js";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {describe} from "vitest";
import {config} from "@lodestar/config/default";
import {Api, ReqTypes} from "../../../../src/beacon/routes/validator.js";
import {getClient} from "../../../../src/beacon/client/validator.js";
Expand Down
6 changes: 3 additions & 3 deletions packages/api/test/unit/beacon/oapiSpec.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import path from "node:path";
import {fileURLToPath} from "node:url";
import {expect} from "chai";
import {describe, it, beforeAll, expect} from "vitest";
import {createChainForkConfig, defaultChainConfig} from "@lodestar/config";
import {OpenApiFile} from "../../utils/parseOpenApiSpec.js";
import {routes} from "../../../src/beacon/index.js";
Expand Down Expand Up @@ -104,7 +104,7 @@ describe("eventstream event data", () => {
const eventstreamExamples =
openApiJson.paths["/eth/v1/events"]["get"].responses["200"].content?.["text/event-stream"].examples;

before("Check eventstreamExamples exists", () => {
beforeAll(() => {
if (!eventstreamExamples) {
throw Error(`eventstreamExamples not defined: ${eventstreamExamples}`);
}
Expand Down Expand Up @@ -136,7 +136,7 @@ describe("eventstream event data", () => {
message: testEvent,
} as routes.events.BeaconEvent);

expect(testEventJson).deep.equals(exampleDataJson, `eventTestData[${topic}] does not match spec's example`);
expect(testEventJson).toEqual(exampleDataJson);
});
}
});
4 changes: 2 additions & 2 deletions packages/api/test/unit/beacon/testData/beacon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import {
} from "../../../../src/beacon/routes/beacon/index.js";
import {GenericServerTestCases} from "../../../utils/genericServerTest.js";

const root = Buffer.alloc(32, 1);
const randao = Buffer.alloc(32, 1);
const root = new Uint8Array(32).fill(1);
const randao = new Uint8Array(32).fill(1);
Comment on lines +12 to +13
Copy link
Contributor Author

Choose a reason for hiding this comment

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

One observation for reviewers, it seems that chai was matching the Buffer and Uint8Array equality. Which is logically wrong, if we are expecting a Buffer instance of a value then assertion should pass otherwise fail.

For this reason have to update a lot of fixture data in this PR, converting Buffer to actual returning Uint8Array instances.

const balance = 32e9;
const pubkeyHex = toHexString(Buffer.alloc(48, 1));

Expand Down
2 changes: 1 addition & 1 deletion packages/api/test/unit/beacon/testData/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export const testData: GenericServerTestCases<Api> = {
res: {
data: {
chainId: 1,
address: Buffer.alloc(20, 1),
address: new Uint8Array(20).fill(1),
},
},
},
Expand Down
2 changes: 1 addition & 1 deletion packages/api/test/unit/beacon/testData/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {Api, EventData, EventType, blobSidecarSSE} from "../../../../src/beacon/
import {GenericServerTestCases} from "../../../utils/genericServerTest.js";

const abortController = new AbortController();
const root = Buffer.alloc(32, 0);
const root = new Uint8Array(32);

/* eslint-disable @typescript-eslint/no-empty-function, @typescript-eslint/naming-convention */

Expand Down
2 changes: 1 addition & 1 deletion packages/api/test/unit/beacon/testData/lightclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,6 @@ export const testData: GenericServerTestCases<Api> = {
},
getCommitteeRoot: {
args: [1, 2],
res: {data: [Buffer.alloc(32, 0), Buffer.alloc(32, 1)]},
res: {data: [Uint8Array.from(Buffer.alloc(32, 0)), Uint8Array.from(Buffer.alloc(32, 1))]},
},
};
38 changes: 28 additions & 10 deletions packages/api/test/unit/beacon/testData/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ import {ssz} from "@lodestar/types";
import {Api} from "../../../../src/beacon/routes/validator.js";
import {GenericServerTestCases} from "../../../utils/genericServerTest.js";

const ZERO_HASH = Buffer.alloc(32, 0);
const ZERO_HASH_HEX = "0x" + ZERO_HASH.toString("hex");
const randaoReveal = Buffer.alloc(96, 1);
const selectionProof = Buffer.alloc(96, 1);
const ZERO_HASH = new Uint8Array(32);
const ZERO_HASH_HEX = "0x" + Buffer.from(ZERO_HASH).toString("hex");
const randaoReveal = new Uint8Array(96).fill(1);
const selectionProof = new Uint8Array(96).fill(1);
const graffiti = "a".repeat(32);
const feeRecipient = "0xbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb";

Expand All @@ -17,7 +17,7 @@ export const testData: GenericServerTestCases<Api> = {
executionOptimistic: true,
data: [
{
pubkey: Buffer.alloc(48, 1),
pubkey: new Uint8Array(48).fill(1),
validatorIndex: 2,
committeeIndex: 3,
committeeLength: 4,
Expand All @@ -33,23 +33,35 @@ export const testData: GenericServerTestCases<Api> = {
args: [1000],
res: {
executionOptimistic: true,
data: [{slot: 1, validatorIndex: 2, pubkey: Buffer.alloc(48, 3)}],
data: [{slot: 1, validatorIndex: 2, pubkey: new Uint8Array(48).fill(3)}],
dependentRoot: ZERO_HASH_HEX,
},
},
getSyncCommitteeDuties: {
args: [1000, [1, 2, 3]],
res: {
executionOptimistic: true,
data: [{pubkey: Buffer.alloc(48, 1), validatorIndex: 2, validatorSyncCommitteeIndices: [3]}],
data: [{pubkey: Uint8Array.from(Buffer.alloc(48, 1)), validatorIndex: 2, validatorSyncCommitteeIndices: [3]}],
},
},
produceBlock: {
args: [32000, randaoReveal, graffiti],
args: [
32000,
randaoReveal,
graffiti,
undefined,
{feeRecipient: undefined, builderSelection: undefined, strictFeeRecipientCheck: undefined},
],
Copy link
Contributor Author

@nazarhussain nazarhussain Dec 11, 2023

Choose a reason for hiding this comment

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

sinon assertions were passing with partial arguments as well. To pass with the vitest we have to add the full arguments that were used to call that function.

That's because we are reusing reqSerializer for produceBlockV3 for other methods as well.

produceBlock: produceBlockV3 as ReqSerializers<Api, ReqTypes>["produceBlock"],
produceBlockV2: produceBlockV3 as ReqSerializers<Api, ReqTypes>["produceBlockV2"],
produceBlockV3,
produceBlindedBlock: produceBlockV3 as ReqSerializers<Api, ReqTypes>["produceBlindedBlock"],

To make it logically correct we should have different serializers for each method. Would be nice to know why it's actually written in this way in first place. @g11tech Do you remember any rationale for it?

Copy link
Contributor

Choose a reason for hiding this comment

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

because produceBlockV3 serializer is superset of all of them

Copy link
Contributor

Choose a reason for hiding this comment

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

you can create separate serializer fns for each of these but then delegate the call to produceBlockV3 inside it with so that you may not need to pass undefined values here

but we anyway will remove these fns and not support them so we can leave it like that as well

res: {data: ssz.phase0.BeaconBlock.defaultValue()},
},
produceBlockV2: {
args: [32000, randaoReveal, graffiti],
args: [
32000,
randaoReveal,
graffiti,
undefined,
{feeRecipient: undefined, builderSelection: undefined, strictFeeRecipientCheck: undefined},
],
res: {
data: ssz.altair.BeaconBlock.defaultValue(),
version: ForkName.altair,
Expand All @@ -74,7 +86,13 @@ export const testData: GenericServerTestCases<Api> = {
},
},
produceBlindedBlock: {
args: [32000, randaoReveal, graffiti],
args: [
32000,
randaoReveal,
graffiti,
undefined,
{feeRecipient: undefined, builderSelection: undefined, strictFeeRecipientCheck: undefined},
],
res: {
data: ssz.bellatrix.BlindedBeaconBlock.defaultValue(),
version: ForkName.bellatrix,
Expand Down
1 change: 1 addition & 0 deletions packages/api/test/unit/builder/builder.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {describe} from "vitest";
import {createChainForkConfig, defaultChainConfig} from "@lodestar/config";
import {Api, ReqTypes} from "../../../src/builder/routes.js";
import {getClient} from "../../../src/builder/client.js";
Expand Down
2 changes: 1 addition & 1 deletion packages/api/test/unit/builder/testData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {GenericServerTestCases} from "../../utils/genericServerTest.js";

// randomly pregenerated pubkey
const pubkeyRand = "0x84105a985058fc8740a48bf1ede9d223ef09e8c6b1735ba0a55cf4a9ff2ff92376b778798365e488dab07a652eb04576";
const root = Buffer.alloc(32, 1);
const root = new Uint8Array(32).fill(1);

export const testData: GenericServerTestCases<Api> = {
status: {
Expand Down
14 changes: 9 additions & 5 deletions packages/api/test/unit/client/fetch.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import crypto from "node:crypto";
import http from "node:http";
import {expect} from "chai";
import {describe, it, expect, afterEach} from "vitest";
import {FetchError, FetchErrorType, fetch} from "../../../src/utils/client/fetch.js";

describe("FetchError", function () {
Expand Down Expand Up @@ -116,12 +116,16 @@ describe("FetchError", function () {
);
}

await expect(fetch(url, {signal: signalHandler?.()})).to.be.rejected.then((error: FetchError) => {
expect(error.type).to.be.equal(testCase.errorType);
expect(error.code).to.be.equal(testCase.errorCode);
await expect(fetch(url, {signal: signalHandler?.()})).rejects.toSatisfy((err) => {
Copy link
Member

Choose a reason for hiding this comment

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

can't this be changed to?

Suggested change
await expect(fetch(url, {signal: signalHandler?.()})).rejects.toSatisfy((err) => {
await expect(fetch(url, {signal: signalHandler?.()})).rejects.toSatisfy((err: FetchError) => {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not that's the strict type issue we can't assign type inside the function descriptor.

Argument of type '(err: FetchError) => true' is not assignable to parameter of type '(value: unknown) => boolean'

expect(err).toBeInstanceOf(FetchError);
expect((err as FetchError).code).toBe(testCase.errorCode);
expect((err as FetchError).type).toBe(testCase.errorType);

if (testCase.expectCause) {
expect(error.cause).to.be.instanceof(Error);
expect((err as FetchError).cause).toBeInstanceOf(Error);
}

return true;
});
});
}
Expand Down
4 changes: 2 additions & 2 deletions packages/api/test/unit/client/format.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import {expect} from "chai";
import {describe, expect, it} from "vitest";
import {EventType} from "../../../src/beacon/routes/events.js";
import {stringifyQuery} from "../../../src/utils/client/format.js";

describe("client / utils / format", () => {
it("Should repeat topic query", () => {
expect(stringifyQuery({topics: [EventType.finalizedCheckpoint]})).to.equal("topics=finalized_checkpoint");
expect(stringifyQuery({topics: [EventType.finalizedCheckpoint]})).toBe("topics=finalized_checkpoint");
});
});
Loading
Loading