From 6d1dc61b621b00581f59cac62f2e836a3d248e77 Mon Sep 17 00:00:00 2001 From: Julien Date: Tue, 19 Dec 2023 06:09:38 -0800 Subject: [PATCH] test: introduce beacon api test ignore list (#6171) * Fix operationId of light_client routes * Fix operationId of bls_to_execution_changes routes * Update beacon api spec version to 2.4.0 * Push case change * Remove now useless file * fix: lints * fix: filter broken test * Revert "Fix operationId of light_client routes" This reverts commit 91cd2af96ad9d4efc2d17a0ed56c1c4229faf8a5. * Revert "Fix operationId of bls_to_execution_changes routes" This reverts commit ad53c2d83caa360a32705a000161fdf1e386d897. * test: ignore missing routes * test: allow to filter required properties from testing * fix: incorrect case * test: fixed incorrect test filtering * fix: lints * fix: cleanup * test: allow more fine grain API tests filtering * fix: lints * test: increase JSON schema validation strictness * fix: restore removed keyword implementation * test: improve filtering semantic * test: add support for JSONPath syntax to filtering * fix: typo Co-authored-by: Nico Flaig * fix: wording Co-authored-by: Nico Flaig * test: improve semantic * test: added issue for context * fix: improved issues references * fix: incorrect dotted property parsing --------- Co-authored-by: Nico Flaig --- .../api/test/unit/beacon/oapiSpec.test.ts | 111 ++++++++++++++++-- packages/api/test/utils/checkAgainstSpec.ts | 79 ++++++++++++- packages/api/test/utils/parseOpenApiSpec.ts | 2 +- 3 files changed, 177 insertions(+), 15 deletions(-) diff --git a/packages/api/test/unit/beacon/oapiSpec.test.ts b/packages/api/test/unit/beacon/oapiSpec.test.ts index c1abd32cb591..1a300eba6f36 100644 --- a/packages/api/test/unit/beacon/oapiSpec.test.ts +++ b/packages/api/test/unit/beacon/oapiSpec.test.ts @@ -6,7 +6,7 @@ import {OpenApiFile} from "../../utils/parseOpenApiSpec.js"; import {routes} from "../../../src/beacon/index.js"; import {ReqSerializers} from "../../../src/utils/types.js"; import {Schema} from "../../../src/utils/schema.js"; -import {runTestCheckAgainstSpec} from "../../utils/checkAgainstSpec.js"; +import {IgnoredProperty, runTestCheckAgainstSpec} from "../../utils/checkAgainstSpec.js"; import {fetchOpenApiSpec} from "../../utils/fetchOpenApiSpec.js"; // Import all testData and merge below import {testData as beaconTestData} from "./testData/beacon.js"; @@ -23,7 +23,7 @@ import {testData as validatorTestData} from "./testData/validator.js"; // eslint-disable-next-line @typescript-eslint/naming-convention const __dirname = path.dirname(fileURLToPath(import.meta.url)); -const version = "v2.3.0"; +const version = "v2.4.2"; const openApiFile: OpenApiFile = { url: `https://github.com/ethereum/beacon-APIs/releases/download/${version}/beacon-node-oapi.json`, filepath: path.join(__dirname, "../../../oapi-schemas/beacon-node-oapi.json"), @@ -84,11 +84,105 @@ const testDatas = { ...validatorTestData, }; +const ignoredOperations = [ + /* missing route */ + /* https://github.com/ChainSafe/lodestar/issues/5694 */ + "getSyncCommitteeRewards", + "getBlockRewards", + "getAttestationsRewards", + "getDepositSnapshot", // Won't fix for now, see https://github.com/ChainSafe/lodestar/issues/5697 + "getBlindedBlock", // https://github.com/ChainSafe/lodestar/issues/5699 + "getNextWithdrawals", // https://github.com/ChainSafe/lodestar/issues/5696 + "getDebugForkChoice", // https://github.com/ChainSafe/lodestar/issues/5700 + /* https://github.com/ChainSafe/lodestar/issues/6080 */ + "getLightClientBootstrap", + "getLightClientUpdatesByRange", + "getLightClientFinalityUpdate", + "getLightClientOptimisticUpdate", + "getPoolBLSToExecutionChanges", + "submitPoolBLSToExecutionChange", +]; + +const ignoredProperties: Record = { + /* + https://github.com/ChainSafe/lodestar/issues/5693 + missing finalized + */ + getStateRoot: {response: ["finalized"]}, + getStateFork: {response: ["finalized"]}, + getStateFinalityCheckpoints: {response: ["finalized"]}, + getStateValidators: {response: ["finalized"]}, + getStateValidator: {response: ["finalized"]}, + getStateValidatorBalances: {response: ["finalized"]}, + getEpochCommittees: {response: ["finalized"]}, + getEpochSyncCommittees: {response: ["finalized"]}, + getStateRandao: {response: ["finalized"]}, + getBlockHeaders: {response: ["finalized"]}, + getBlockHeader: {response: ["finalized"]}, + getBlockV2: {response: ["finalized"]}, + getBlockRoot: {response: ["finalized"]}, + getBlockAttestations: {response: ["finalized"]}, + getStateV2: {response: ["finalized"]}, + + /* + https://github.com/ChainSafe/lodestar/issues/6168 + /query/syncing_status - must be integer + */ + getHealth: {request: ["query.syncing_status"]}, + + /** + * https://github.com/ChainSafe/lodestar/issues/6185 + * - must have required property 'query' + */ + getBlobSidecars: {request: ["query"]}, + + /* + https://github.com/ChainSafe/lodestar/issues/4638 + /query - must have required property 'skip_randao_verification' + */ + produceBlockV2: {request: ["query.skip_randao_verification"]}, + produceBlindedBlock: {request: ["query.skip_randao_verification"]}, +}; + const openApiJson = await fetchOpenApiSpec(openApiFile); -runTestCheckAgainstSpec(openApiJson, routesData, reqSerializers, returnTypes, testDatas, { - // TODO: Investigate why schema validation fails otherwise - routesDropOneOf: ["produceBlockV2", "produceBlindedBlock", "publishBlindedBlock"], -}); +runTestCheckAgainstSpec( + openApiJson, + routesData, + reqSerializers, + returnTypes, + testDatas, + { + // TODO: Investigate why schema validation fails otherwise (see https://github.com/ChainSafe/lodestar/issues/6187) + routesDropOneOf: [ + "produceBlockV2", + "produceBlockV3", + "produceBlindedBlock", + "publishBlindedBlock", + "publishBlindedBlockV2", + ], + }, + ignoredOperations, + ignoredProperties +); + +const ignoredTopics = [ + /* + https://github.com/ChainSafe/lodestar/issues/6167 + eventTestData[bls_to_execution_change] does not match spec's example + */ + "bls_to_execution_change", + /* + https://github.com/ChainSafe/lodestar/issues/6170 + Error: Invalid slot=0 fork=phase0 for lightclient fork types + */ + "light_client_finality_update", + "light_client_optimistic_update", + /* + https://github.com/ethereum/beacon-APIs/pull/379 + SyntaxError: Unexpected non-whitespace character after JSON at position 629 (line 1 column 630) + */ + "payload_attributes", +]; // eventstream types are defined as comments in the description of "examples". // The function runTestCheckAgainstSpec() can't handle those, so the custom code before: @@ -113,7 +207,9 @@ describe("eventstream event data", () => { const eventSerdes = routes.events.getEventSerdes(config); const knownTopics = new Set(Object.values(routes.events.eventTypes)); - for (const [topic, {value}] of Object.entries(eventstreamExamples ?? {})) { + for (const [topic, {value}] of Object.entries(eventstreamExamples ?? {}).filter( + ([topic]) => !ignoredTopics.includes(topic) + )) { it(topic, () => { if (!knownTopics.has(topic)) { throw Error(`topic ${topic} not implemented`); @@ -130,7 +226,6 @@ describe("eventstream event data", () => { if (testEvent == null) { throw Error(`No eventTestData for ${topic}`); } - const testEventJson = eventSerdes.toJson({ type: topic as routes.events.EventType, message: testEvent, diff --git a/packages/api/test/utils/checkAgainstSpec.ts b/packages/api/test/utils/checkAgainstSpec.ts index 01e7df255db2..ed65279bca22 100644 --- a/packages/api/test/utils/checkAgainstSpec.ts +++ b/packages/api/test/utils/checkAgainstSpec.ts @@ -1,16 +1,16 @@ import Ajv, {ErrorObject} from "ajv"; import {expect, describe, beforeAll, it} from "vitest"; import {ReqGeneric, ReqSerializer, ReturnTypes, RouteDef} from "../../src/utils/types.js"; -import {applyRecursively, OpenApiJson, parseOpenApiSpec, ParseOpenApiSpecOpts} from "./parseOpenApiSpec.js"; +import {applyRecursively, JsonSchema, OpenApiJson, parseOpenApiSpec, ParseOpenApiSpecOpts} from "./parseOpenApiSpec.js"; import {GenericServerTestCases} from "./genericServerTest.js"; const ajv = new Ajv({ - // strict: true, - // strictSchema: true, + strict: true, + strictTypes: false, // TODO Enable once beacon-APIs is fixed. See https://github.com/ChainSafe/lodestar/issues/6206 allErrors: true, }); -// TODO: Still necessary? +// Ensure embedded schema 'example' do not fail validation ajv.addKeyword({ keyword: "example", validate: () => true, @@ -19,17 +19,69 @@ ajv.addKeyword({ ajv.addFormat("hex", /^0x[a-fA-F0-9]+$/); +/** + * A set of properties that will be ignored during tests execution. + * This allows for a black-list mechanism to have a test pass while some part of the spec is not yet implemented. + * + * Properties can be nested using dot notation, following JSONPath semantic. + * + * Example: + * - query + * - query.skip_randao_verification + */ +export type IgnoredProperty = { + /** + * Properties to ignore in the request schema + */ + request?: string[]; + /** + * Properties to ignore in the response schema + */ + response?: string[]; +}; + +/** + * Recursively remove a property from a schema + * + * @param schema Schema to remove a property from + * @param property JSONPath like property to remove from the schema + */ +function deleteNested(schema: JsonSchema | undefined, property: string): void { + const properties = schema?.properties; + if (property.includes(".")) { + // Extract first segment, keep the rest as dotted + const [key, ...rest] = property.split("."); + deleteNested(properties?.[key], rest.join(".")); + } else { + // Remove property from 'required' + if (schema?.required) { + schema.required = schema.required?.filter((e) => property !== e); + } + // Remove property from 'properties' + delete properties?.[property]; + } +} + export function runTestCheckAgainstSpec( openApiJson: OpenApiJson, routesData: Record, reqSerializers: Record>, returnTypes: Record[string]>, testDatas: Record[string]>, - opts?: ParseOpenApiSpecOpts + opts?: ParseOpenApiSpecOpts, + ignoredOperations: string[] = [], + ignoredProperties: Record = {} ): void { const openApiSpec = parseOpenApiSpec(openApiJson, opts); for (const [operationId, routeSpec] of openApiSpec.entries()) { + const isIgnored = ignoredOperations.some((id) => id === operationId); + if (isIgnored) { + continue; + } + + const ignoredProperty = ignoredProperties[operationId]; + describe(operationId, () => { const {requestSchema, responseOkSchema} = routeSpec; const routeId = operationId; @@ -68,7 +120,15 @@ export function runTestCheckAgainstSpec( stringifyProperties((reqJson as ReqGeneric).params ?? {}); stringifyProperties((reqJson as ReqGeneric).query ?? {}); - // Validate response + const ignoredProperties = ignoredProperty?.request; + if (ignoredProperties) { + // Remove ignored properties from schema validation + for (const property of ignoredProperties) { + deleteNested(routeSpec.requestSchema, property); + } + } + + // Validate request validateSchema(routeSpec.requestSchema, reqJson, "request"); }); } @@ -87,6 +147,13 @@ export function runTestCheckAgainstSpec( } } + const ignoredProperties = ignoredProperty?.response; + if (ignoredProperties) { + // Remove ignored properties from schema validation + for (const property of ignoredProperties) { + deleteNested(routeSpec.responseOkSchema, property); + } + } // Validate response validateSchema(responseOkSchema, resJson, "response"); }); diff --git a/packages/api/test/utils/parseOpenApiSpec.ts b/packages/api/test/utils/parseOpenApiSpec.ts index 5faf0082012d..84b024e5950e 100644 --- a/packages/api/test/utils/parseOpenApiSpec.ts +++ b/packages/api/test/utils/parseOpenApiSpec.ts @@ -11,7 +11,7 @@ type RouteUrl = string; /** "get" | "post" */ type HttpMethod = string; -type JsonSchema = { +export type JsonSchema = { type: "object"; properties?: Record; required?: string[];