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 91cd2af. * Revert "Fix operationId of bls_to_execution_changes routes" This reverts commit ad53c2d. * 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 <nflaig@protonmail.com> * fix: wording Co-authored-by: Nico Flaig <nflaig@protonmail.com> * test: improve semantic * test: added issue for context * fix: improved issues references * fix: incorrect dotted property parsing --------- Co-authored-by: Nico Flaig <nflaig@protonmail.com>
ChainSafe · Dec 19, 2023 · 6d1dc61 · 6d1dc61
1 parent 7e85f10
commit 6d1dc61
Show file tree

Hide file tree

Showing 3 changed files with 177 additions and 15 deletions.
diff --git 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<string, IgnoredProperty> = {
+  /*
+   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<string>(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
@@ -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<string, RouteDef>,
   reqSerializers: Record<string, ReqSerializer<any, any>>,
   returnTypes: Record<string, ReturnTypes<any>[string]>,
   testDatas: Record<string, GenericServerTestCases<any>[string]>,
-  opts?: ParseOpenApiSpecOpts
+  opts?: ParseOpenApiSpecOpts,
+  ignoredOperations: string[] = [],
+  ignoredProperties: Record<string, IgnoredProperty> = {}
 ): 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
@@ -11,7 +11,7 @@ type RouteUrl = string;
 /** "get" | "post" */
 type HttpMethod = string;
 
-type JsonSchema = {
+export type JsonSchema = {
   type: "object";
   properties?: Record<string, JsonSchema>;
   required?: string[];