* feat!: update dep, TS arg validation, stricter internal TS

* feat: add Typescript hints for the `filepathOrObject` arg * refactor: much stricter internal use of TypeScript * BREAKING CHANGE: this updates [our validation dependency](https://www.npmjs.com/package/openapi-response-validator) by 2 major versions, so validation error messages (and maybe validation rules) are slightly different. For example, `res did not satisfy it because: property1 should be string, property2 should be string` is now `res did not satisfy it because: property1 must be string, property2 must be string` (`should` -> `must`). We expect our users to _read_ these error messages during testing but not assert on them, such that this change shouldn't break anyone's existing tests. Moreover, renaming `should` to `must` does not indicate that the validation rule has changed. **However, it is possible that _other_ validation rules have changed**
openapi-library · Sep 12, 2021 · 04b3d88 · 04b3d88
1 parent f9a9767
commit 04b3d88
Show file tree

Hide file tree

Showing 36 changed files with 782 additions and 455 deletions.
diff --git a/.eslintrc.yml b/.eslintrc.yml
@@ -11,7 +11,7 @@ env:
 extends:
   - eslint:recommended
   - airbnb-typescript/base
-  # - plugin:@typescript-eslint/recommended
+  - plugin:@typescript-eslint/recommended
   # - plugin:@typescript-eslint/recommended-requiring-type-checking
   - prettier/@typescript-eslint
   - prettier # must go last, to turn off some previous rules

diff --git a/packages/chai-openapi-response-validator/lib/assertions/satisfyApiSpec.ts b/packages/chai-openapi-response-validator/lib/assertions/satisfyApiSpec.ts
@@ -1,8 +1,18 @@
-import { makeResponse } from 'openapi-validator';
-
-import { stringify, joinWithNewLines } from '../utils';
-
-export default function (chai, openApiSpec) {
+import {
+  ActualResponse,
+  ErrorCode,
+  makeResponse,
+  OpenApi2Spec,
+  OpenApi3Spec,
+  OpenApiSpec,
+  ValidationError,
+} from 'openapi-validator';
+import { joinWithNewLines, stringify } from '../utils';
+
+export default function (
+  chai: Chai.ChaiStatic,
+  openApiSpec: OpenApiSpec,
+): void {
   const { Assertion } = chai;
 
   Assertion.addProperty('satisfyApiSpec', function () {
@@ -21,15 +31,16 @@ export default function (chai, openApiSpec) {
         openApiSpec,
         validationError,
       ),
+      null,
     );
   });
 }
 
 function getExpectedResToSatisfyApiSpecMsg(
-  actualResponse,
-  openApiSpec,
-  validationError,
-) {
+  actualResponse: ActualResponse,
+  openApiSpec: OpenApiSpec,
+  validationError: ValidationError,
+): string | null {
   if (!validationError) {
     return null;
   }
@@ -39,39 +50,49 @@ function getExpectedResToSatisfyApiSpecMsg(
   const { method, path: requestPath } = req;
   const unmatchedEndpoint = `${method} ${requestPath}`;
 
-  if (validationError.code === `SERVER_NOT_FOUND`) {
+  if (validationError.code === ErrorCode.ServerNotFound) {
     return joinWithNewLines(
       hint,
       `expected res to satisfy a '${status}' response defined for endpoint '${unmatchedEndpoint}' in your API spec`,
       `res had request path '${requestPath}', but your API spec has no matching servers`,
-      `Servers found in API spec: ${openApiSpec.getServerUrls().join(', ')}`,
+      `Servers found in API spec: ${(openApiSpec as OpenApi3Spec)
+        .getServerUrls()
+        .join(', ')}`,
     );
   }
 
-  if (validationError.code === `BASE_PATH_NOT_FOUND`) {
+  if (validationError.code === ErrorCode.BasePathNotFound) {
     return joinWithNewLines(
       hint,
       `expected res to satisfy a '${status}' response defined for endpoint '${unmatchedEndpoint}' in your API spec`,
-      `res had request path '${requestPath}', but your API spec has basePath '${openApiSpec.spec.basePath}'`,
+      `res had request path '${requestPath}', but your API spec has basePath '${
+        (openApiSpec as OpenApi2Spec).spec.basePath
+      }'`,
     );
   }
 
-  if (validationError.code === `PATH_NOT_FOUND`) {
+  if (validationError.code === ErrorCode.PathNotFound) {
     const pathNotFoundErrorMessage = joinWithNewLines(
       hint,
       `expected res to satisfy a '${status}' response defined for endpoint '${unmatchedEndpoint}' in your API spec`,
       `res had request path '${requestPath}', but your API spec has no matching path`,
       `Paths found in API spec: ${openApiSpec.paths().join(', ')}`,
     );
 
-    if (openApiSpec.didUserDefineBasePath) {
+    if (
+      'didUserDefineBasePath' in openApiSpec &&
+      openApiSpec.didUserDefineBasePath
+    ) {
       return joinWithNewLines(
         pathNotFoundErrorMessage,
         `'${requestPath}' matches basePath \`${openApiSpec.spec.basePath}\` but no <basePath/endpointPath> combinations`,
       );
     }
 
-    if (openApiSpec.didUserDefineServers) {
+    if (
+      'didUserDefineServers' in openApiSpec &&
+      openApiSpec.didUserDefineServers
+    ) {
       return joinWithNewLines(
         pathNotFoundErrorMessage,
         `'${requestPath}' matches servers ${stringify(
@@ -85,7 +106,7 @@ function getExpectedResToSatisfyApiSpecMsg(
   const path = openApiSpec.findOpenApiPathMatchingRequest(req);
   const endpoint = `${method} ${path}`;
 
-  if (validationError.code === 'METHOD_NOT_FOUND') {
+  if (validationError.code === ErrorCode.MethodNotFound) {
     const expectedPathItem = openApiSpec.findExpectedPathItem(req);
     const expectedRequestOperations = Object.keys(expectedPathItem)
       .map((operation) => operation.toUpperCase())
@@ -98,7 +119,7 @@ function getExpectedResToSatisfyApiSpecMsg(
     );
   }
 
-  if (validationError.code === 'STATUS_NOT_FOUND') {
+  if (validationError.code === ErrorCode.StatusNotFound) {
     const expectedResponseOperation = openApiSpec.findExpectedResponseOperation(
       req,
     );
@@ -113,7 +134,7 @@ function getExpectedResToSatisfyApiSpecMsg(
     );
   }
 
-  // validationError.code === 'INVALID_BODY'
+  // validationError.code === ErrorCode.InvalidBody
   const responseDefinition = openApiSpec.findExpectedResponse(actualResponse);
   return joinWithNewLines(
     hint,
@@ -127,10 +148,10 @@ function getExpectedResToSatisfyApiSpecMsg(
 }
 
 function getExpectedResNotToSatisfyApiSpecMsg(
-  actualResponse,
-  openApiSpec,
+  actualResponse: ActualResponse,
+  openApiSpec: OpenApiSpec,
   validationError,
-) {
+): string | null {
   if (validationError) {
     return null;
   }

diff --git a/packages/chai-openapi-response-validator/lib/assertions/satisfySchemaInApiSpec.ts b/packages/chai-openapi-response-validator/lib/assertions/satisfySchemaInApiSpec.ts
@@ -1,6 +1,10 @@
+import type { OpenApiSpec, Schema, ValidationError } from 'openapi-validator';
 import { stringify, joinWithNewLines } from '../utils';
 
-export default function (chai, openApiSpec) {
+export default function (
+  chai: Chai.ChaiStatic,
+  openApiSpec: OpenApiSpec,
+): void {
   const { Assertion, AssertionError } = chai;
 
   Assertion.addMethod('satisfySchemaInApiSpec', function (schemaName) {
@@ -9,6 +13,7 @@ export default function (chai, openApiSpec) {
     const schema = openApiSpec.getSchemaObject(schemaName);
     if (!schema) {
       // alert users they are misusing this assertion
+      // eslint-disable-next-line @typescript-eslint/no-throw-literal
       throw new AssertionError(
         'The argument to satisfySchemaInApiSpec must match a schema in your API spec',
       );
@@ -29,32 +34,33 @@ export default function (chai, openApiSpec) {
         schemaName,
         schema,
       ),
+      null,
     );
   });
 }
 
 function getExpectReceivedToSatisfySchemaInApiSpecMsg(
-  actualObject,
-  schemaName,
-  schema,
-  validationError,
+  received: unknown,
+  schemaName: string,
+  schema: Schema,
+  validationError: ValidationError,
 ) {
   return joinWithNewLines(
     `expected object to satisfy the '${schemaName}' schema defined in your API spec`,
     `object did not satisfy it because: ${validationError}`,
-    `object was: ${stringify(actualObject)}`,
+    `object was: ${stringify(received)}`,
     `The '${schemaName}' schema in API spec: ${stringify(schema)}`,
   );
 }
 
 function getExpectReceivedNotToSatisfySchemaInApiSpecMsg(
-  actualObject,
-  schemaName,
-  schema,
+  received: unknown,
+  schemaName: string,
+  schema: Schema,
 ) {
   return joinWithNewLines(
     `expected object not to satisfy the '${schemaName}' schema defined in your API spec`,
-    `object was: ${stringify(actualObject)}`,
+    `object was: ${stringify(received)}`,
     `The '${schemaName}' schema in API spec: ${stringify(schema)}`,
   );
 }
diff --git a/packages/chai-openapi-response-validator/lib/index.ts b/packages/chai-openapi-response-validator/lib/index.ts
@@ -1,9 +1,9 @@
-import { makeApiSpec } from 'openapi-validator';
-
+import { makeApiSpec, OpenAPISpecObject } from 'openapi-validator';
 import satisfyApiSpec from './assertions/satisfyApiSpec';
 import satisfySchemaInApiSpec from './assertions/satisfySchemaInApiSpec';
 
 declare global {
+  // eslint-disable-next-line @typescript-eslint/no-namespace
   namespace Chai {
     interface Assertion {
       /**
@@ -20,7 +20,9 @@ declare global {
   }
 }
 
-export default function (filepathOrObject: string | object): Chai.ChaiPlugin {
+export default function (
+  filepathOrObject: string | OpenAPISpecObject,
+): Chai.ChaiPlugin {
   const openApiSpec = makeApiSpec(filepathOrObject);
   return function (chai) {
     satisfyApiSpec(chai, openApiSpec);

diff --git a/packages/chai-openapi-response-validator/lib/utils.ts b/packages/chai-openapi-response-validator/lib/utils.ts
@@ -1,6 +1,7 @@
 import { inspect } from 'util';
 
-export const stringify = (obj) =>
+export const stringify = (obj: unknown): string =>
   inspect(obj, { showHidden: false, depth: null });
 
-export const joinWithNewLines = (...lines) => lines.join('\n\n');
+export const joinWithNewLines = (...lines: string[]): string =>
+  lines.join('\n\n');
diff --git a/...ges/chai-openapi-response-validator/test/assertions/satisfyApiSpec/satisfyApiSpec.test.ts b/...ges/chai-openapi-response-validator/test/assertions/satisfyApiSpec/satisfyApiSpec.test.ts
@@ -33,7 +33,7 @@ openApiSpecs.forEach((spec) => {
     describe("when 'res' is not a valid HTTP response object", () => {
       const res = {
         status: 204,
-        body: "should have a 'path' property",
+        body: "must have a 'path' property",
       };
 
       it('fails', () => {
@@ -496,7 +496,7 @@ openApiSpecs.forEach((spec) => {
             expect(assertion).to.throw(
               joinWithNewLines(
                 "expected res to satisfy the '200' response defined for endpoint 'GET /responseBody/object/withMultipleProperties' in your API spec",
-                'res did not satisfy it because: property1 should be string, property2 should be string',
+                'res did not satisfy it because: property1 must be string, property2 must be string',
                 `res contained: ${str({
                   body: { property1: 123, property2: 123 },
                 })}`,

diff --git a/...enapi-response-validator/test/assertions/satisfyApiSpec/serversDefinedDifferently.test.ts b/...enapi-response-validator/test/assertions/satisfyApiSpec/serversDefinedDifferently.test.ts
@@ -57,10 +57,12 @@ describe('Using OpenAPI 3 specs that define servers differently', () => {
       it('fails', () => {
         const assertion = () => expect(res).to.satisfyApiSpec;
         expect(assertion).to.throw(
-          joinWithNewLines(
-            "expected res to satisfy a '200' response defined for endpoint 'GET /nonExistentEndpointPath' in your API spec",
-            "res had request path '/nonExistentEndpointPath', but your API spec has no matching path",
-            'Paths found in API spec:',
+          new RegExp(
+            `${joinWithNewLines(
+              "expected res to satisfy a '200' response defined for endpoint 'GET /nonExistentEndpointPath' in your API spec",
+              "res had request path '/nonExistentEndpointPath', but your API spec has no matching path",
+              'Paths found in API spec: /endpointPath',
+            )}$`,
           ),
         );
       });
@@ -149,11 +151,13 @@ describe('Using OpenAPI 3 specs that define servers differently', () => {
       it('fails', () => {
         const assertion = () => expect(res).to.satisfyApiSpec;
         expect(assertion).to.throw(
-          joinWithNewLines(
-            expectedResToSatisfyApiSpec,
-            "expected res to satisfy a '200' response defined for endpoint 'GET /nonExistentServer' in your API spec",
-            "res had request path '/nonExistentServer', but your API spec has no matching servers",
-            'Servers found in API spec: /relativeServer, /differentRelativeServer, /relativeServer2, http://api.example.com/basePath1, https://api.example.com/basePath2, ws://api.example.com/basePath3, wss://api.example.com/basePath4, http://api.example.com:8443/basePath5, http://localhost:3025/basePath6, http://10.0.81.36/basePath7',
+          new RegExp(
+            `${joinWithNewLines(
+              expectedResToSatisfyApiSpec,
+              "expected res to satisfy a '200' response defined for endpoint 'GET /nonExistentServer' in your API spec",
+              "res had request path '/nonExistentServer', but your API spec has no matching servers",
+              'Servers found in API spec: /relativeServer, /differentRelativeServer, /relativeServer2, http://api.example.com/basePath1, https://api.example.com/basePath2, ws://api.example.com/basePath3, wss://api.example.com/basePath4, http://api.example.com:8443/basePath5, http://localhost:3025/basePath6, http://10.0.81.36/basePath7',
+            )}$`,
           ),
         );
       });

diff --git a/...-response-validator/test/assertions/satisfySchemaInApiSpec/satisfySchemaInApiSpec.test.ts b/...-response-validator/test/assertions/satisfySchemaInApiSpec/satisfySchemaInApiSpec.test.ts
@@ -67,7 +67,7 @@ openApiSpecs.forEach((spec) => {
             expect(assertion).to.throw(
               joinWithNewLines(
                 `expected object to satisfy the '${schemaName}' schema defined in your API spec`,
-                'object did not satisfy it because: object should be string',
+                'object did not satisfy it because: object must be string',
                 'object was: 123',
                 `The '${schemaName}' schema in API spec: ${str(
                   expectedSchema,
@@ -108,15 +108,15 @@ openApiSpecs.forEach((spec) => {
         });
 
         describe("'obj' does not satisfy the spec", () => {
-          const invalidObj = 'should be integer';
+          const invalidObj = 'must be integer';
 
           it('fails and outputs a useful error message', () => {
             const assertion = () =>
               expect(invalidObj).to.satisfySchemaInApiSpec(schemaName);
             expect(assertion).to.throw(
               joinWithNewLines(
-                'object did not satisfy it because: object should be integer',
-                "object was: 'should be integer'",
+                'object did not satisfy it because: object must be integer',
+                "object was: 'must be integer'",
                 `The '${schemaName}' schema in API spec: ${str(
                   expectedSchema,
                 )}`,
@@ -168,7 +168,7 @@ openApiSpecs.forEach((spec) => {
             expect(assertion).to.throw(
               AssertionError,
               joinWithNewLines(
-                `object did not satisfy it because: property1 should be string`,
+                `object did not satisfy it because: property1 must be string`,
                 `object was: ${str(invalidObj)}`,
                 `The '${schemaName}' schema in API spec: ${str(
                   expectedSchema,
@@ -224,7 +224,7 @@ openApiSpecs.forEach((spec) => {
               expect(invalidObj).to.satisfySchemaInApiSpec(schemaName);
             expect(assertion).to.throw(
               joinWithNewLines(
-                `object did not satisfy it because: property1 should be string`,
+                `object did not satisfy it because: property1 must be string`,
                 `object was: ${str(invalidObj)}`,
                 `The '${schemaName}' schema in API spec: ${str(
                   expectedSchema,
@@ -267,7 +267,7 @@ openApiSpecs.forEach((spec) => {
               expect(invalidObj).to.satisfySchemaInApiSpec(schemaName);
             expect(assertion).to.throw(
               AssertionError,
-              'object did not satisfy it because: property2 should be string',
+              'object did not satisfy it because: property2 must be string',
             );
           });
 
@@ -306,7 +306,7 @@ openApiSpecs.forEach((spec) => {
                 expect(invalidObj).to.satisfySchemaInApiSpec(schemaName);
               expect(assertion).to.throw(
                 AssertionError,
-                'object did not satisfy it because: property1 should be string, property2 should be string, object should match some schema in anyOf',
+                'object did not satisfy it because: property1 must be string, property2 must be string, object must match a schema in anyOf',
               );
             });
 
@@ -344,7 +344,7 @@ openApiSpecs.forEach((spec) => {
                 expect(invalidObj).to.satisfySchemaInApiSpec(schemaName);
               expect(assertion).to.throw(
                 AssertionError,
-                'object did not satisfy it because: object should match exactly one schema in oneOf',
+                'object did not satisfy it because: object must match exactly one schema in oneOf',
               );
             });