From f2afd7171406c7477fbd644d74087bb0e2948c75 Mon Sep 17 00:00:00 2001 From: "Daniel (dB.) Doubrovkine" Date: Wed, 19 Jun 2024 18:46:31 -0400 Subject: [PATCH] Check response payload. (#347) Signed-off-by: dblock --- CHANGELOG.md | 1 + package-lock.json | 10 ++++++ package.json | 1 + tests/_core/info.yaml | 4 +++ tools/src/tester/ChapterEvaluator.ts | 33 ++++++++++++++++--- tools/src/tester/ResultLogger.ts | 12 +++++-- tools/src/tester/types/eval.types.ts | 3 +- .../fixtures/evals/error/chapter_error.yaml | 4 ++- .../fixtures/evals/failed/invalid_data.yaml | 15 ++++++--- .../fixtures/evals/failed/not_found.yaml | 12 +++++-- tools/tests/tester/fixtures/evals/passed.yaml | 4 ++- .../fixtures/stories/failed/invalid_data.yaml | 5 +-- tools/tests/tester/helpers.ts | 3 +- 13 files changed, 86 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a427bf29..53beddde3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Added `distribution` field to `OpenSearchVersionInfo` ([#336](https://github.com/opensearch-project/opensearch-api-specification/pull/336)) - Added `created_time` and `last_updated_time` to `ml.get_model_group@200` ([#342](https://github.com/opensearch-project/opensearch-api-specification/pull/342)) - Added spellcheck linter ([#341](https://github.com/opensearch-project/opensearch-api-specification/pull/341)) +- Added tests for response payload ([#347](https://github.com/opensearch-project/opensearch-api-specification/pull/347)) ### Changed diff --git a/package-lock.json b/package-lock.json index f0ca22c86..efb49a279 100644 --- a/package-lock.json +++ b/package-lock.json @@ -39,6 +39,7 @@ "eslint-plugin-yml": "^1.14.0", "globals": "^15.0.0", "jest": "^29.7.0", + "json-diff-ts": "^4.0.1", "json-schema-to-typescript": "^14.0.4", "ts-jest": "^29.1.2" } @@ -6109,6 +6110,15 @@ "integrity": "sha512-4bV5BfR2mqfQTJm+V5tPPdf+ZpuhiIvTuAB5g8kcrXOZpTT/QwwVRWBywX1ozr6lEuPdbHxwaJlm9G6mI2sfSQ==", "dev": true }, + "node_modules/json-diff-ts": { + "version": "4.0.1", + "resolved": "https://registry.npmjs.org/json-diff-ts/-/json-diff-ts-4.0.1.tgz", + "integrity": "sha512-FEuq+gv4DXI+nL7oF/trBQIBQYDVcJe5Kqe3mYUZAkDtHBY/cdWmVZpV9BLZFp+wThMClajYp0fmNVmB81FsmA==", + "dev": true, + "dependencies": { + "lodash": "4.x" + } + }, "node_modules/json-parse-even-better-errors": { "version": "2.3.1", "resolved": "https://registry.npmjs.org/json-parse-even-better-errors/-/json-parse-even-better-errors-2.3.1.tgz", diff --git a/package.json b/package.json index b0d5f6ae4..ddcd4a510 100644 --- a/package.json +++ b/package.json @@ -48,6 +48,7 @@ "eslint-plugin-yml": "^1.14.0", "globals": "^15.0.0", "jest": "^29.7.0", + "json-diff-ts": "^4.0.1", "json-schema-to-typescript": "^14.0.4", "ts-jest": "^29.1.2" } diff --git a/tests/_core/info.yaml b/tests/_core/info.yaml index 5a8e14f07..d46ca3eae 100644 --- a/tests/_core/info.yaml +++ b/tests/_core/info.yaml @@ -16,3 +16,7 @@ chapters: pretty: false response: status: 200 + payload: + version: + distribution: opensearch + tagline: 'The OpenSearch Project: https://opensearch.org/' diff --git a/tools/src/tester/ChapterEvaluator.ts b/tools/src/tester/ChapterEvaluator.ts index fefa0202f..2ed0cc661 100644 --- a/tools/src/tester/ChapterEvaluator.ts +++ b/tools/src/tester/ChapterEvaluator.ts @@ -7,7 +7,7 @@ * compatible open source license. */ -import { type Chapter, type ActualResponse } from './types/story.types' +import { type Chapter, type ActualResponse, type Payload } from './types/story.types' import { type ChapterEvaluation, type Evaluation, Result } from './types/eval.types' import { type ParsedOperation } from './types/spec.types' import { overall_result } from './helpers' @@ -16,6 +16,8 @@ import type OperationLocator from './OperationLocator' import type SchemaValidator from './SchemaValidator' import { type StoryOutputs } from './StoryOutputs' import { ChapterOutput } from './ChapterOutput' +import { Operation, atomizeChangeset, diff } from 'json-diff-ts' +import _ from 'lodash' export default class ChapterEvaluator { private readonly _operation_locator: OperationLocator @@ -36,13 +38,20 @@ export default class ChapterEvaluator { const params = this.#evaluate_parameters(chapter, operation) const request_body = this.#evaluate_request_body(chapter, operation) const status = this.#evaluate_status(chapter, response) - const payload = status.result === Result.PASSED ? this.#evaluate_payload(response, operation) : { result: Result.SKIPPED } + const payload_body_evaluation = status.result === Result.PASSED ? this.#evaluate_payload_body(response, chapter.response?.payload) : { result: Result.SKIPPED } + const payload_schema_evaluation = status.result === Result.PASSED ? this.#evaluate_payload_schema(response, operation) : { result: Result.SKIPPED } const output_values = ChapterOutput.extract_output_values(response, chapter.output) return { title: chapter.synopsis, - overall: { result: overall_result(Object.values(params).concat([request_body, status, payload]).concat(output_values ? [output_values] : [])) }, + overall: { result: overall_result(Object.values(params).concat([ + request_body, status, payload_body_evaluation, payload_schema_evaluation + ]).concat(output_values ? [output_values] : [])) }, request: { parameters: params, request_body }, - response: { status, payload }, + response: { + status, + payload_body: payload_body_evaluation, + payload_schema: payload_schema_evaluation + }, ...(output_values ? { output_values } : {}) } } @@ -74,7 +83,21 @@ export default class ChapterEvaluator { } } - #evaluate_payload(response: ActualResponse, operation: ParsedOperation): Evaluation { + #evaluate_payload_body(response: ActualResponse, expected_payload?: Payload): Evaluation { + if (expected_payload == null) return { result: Result.PASSED } + const delta = atomizeChangeset(diff(expected_payload, response.payload)) + const messages: string[] = _.compact(delta.map((value, _index, _array) => { + switch(value.type) { + case Operation.UPDATE: + return `expected ${value.path.replace('$.', '')}='${value.oldValue}', got '${value.value}'` + case Operation.REMOVE: + return `missing ${value.path.replace('$.', '')}='${value.value}'` + } + })) + return messages.length > 0 ? { result: Result.FAILED, message: _.join(messages, ', ')} : { result: Result.PASSED } + } + + #evaluate_payload_schema(response: ActualResponse, operation: ParsedOperation): Evaluation { const content_type = response.content_type ?? 'application/json' const content = operation.responses[response.status]?.content[content_type] const schema = content?.schema diff --git a/tools/src/tester/ResultLogger.ts b/tools/src/tester/ResultLogger.ts index 1707eab58..2a1aef7fe 100644 --- a/tools/src/tester/ResultLogger.ts +++ b/tools/src/tester/ResultLogger.ts @@ -53,7 +53,8 @@ export class ConsoleResultLogger implements ResultLogger { this.#log_parameters(chapter.request?.parameters ?? {}) this.#log_request_body(chapter.request?.request_body) this.#log_status(chapter.response?.status) - this.#log_payload(chapter.response?.payload) + this.#log_payload_body(chapter.response?.payload_body) + this.#log_payload_schema(chapter.response?.payload_schema) } #log_parameters (parameters: Record): void { @@ -75,9 +76,14 @@ export class ConsoleResultLogger implements ResultLogger { this.#log_evaluation(evaluation, 'RESPONSE STATUS', this._tab_width * 3) } - #log_payload (evaluation: Evaluation | undefined): void { + #log_payload_body (evaluation: Evaluation | undefined): void { if (evaluation == null) return - this.#log_evaluation(evaluation, 'RESPONSE PAYLOAD', this._tab_width * 3) + this.#log_evaluation(evaluation, 'RESPONSE PAYLOAD BODY', this._tab_width * 3) + } + + #log_payload_schema (evaluation: Evaluation | undefined): void { + if (evaluation == null) return + this.#log_evaluation(evaluation, 'RESPONSE PAYLOAD SCHEMA', this._tab_width * 3) } #log_evaluation (evaluation: Evaluation, title: string, prefix: number = 0): void { diff --git a/tools/src/tester/types/eval.types.ts b/tools/src/tester/types/eval.types.ts index e4c4fb160..d42e39912 100644 --- a/tools/src/tester/types/eval.types.ts +++ b/tools/src/tester/types/eval.types.ts @@ -32,7 +32,8 @@ export interface ChapterEvaluation { } response?: { status: Evaluation - payload: Evaluation + payload_body: Evaluation, + payload_schema: Evaluation } output_values?: EvaluationWithOutput } diff --git a/tools/tests/tester/fixtures/evals/error/chapter_error.yaml b/tools/tests/tester/fixtures/evals/error/chapter_error.yaml index 2a11f2e54..669fefd08 100644 --- a/tools/tests/tester/fixtures/evals/error/chapter_error.yaml +++ b/tools/tests/tester/fixtures/evals/error/chapter_error.yaml @@ -27,7 +27,9 @@ chapters: message: 'Expected status 200, but received 404: application/json. no such index [undefined]' error: Request failed with status code 404 - payload: + payload_body: + result: SKIPPED + payload_schema: result: SKIPPED - title: This chapter should be skipped. overall: diff --git a/tools/tests/tester/fixtures/evals/failed/invalid_data.yaml b/tools/tests/tester/fixtures/evals/failed/invalid_data.yaml index 1cc11180a..3bf4abbac 100644 --- a/tools/tests/tester/fixtures/evals/failed/invalid_data.yaml +++ b/tools/tests/tester/fixtures/evals/failed/invalid_data.yaml @@ -20,7 +20,9 @@ chapters: response: status: result: PASSED - payload: + payload_body: + result: PASSED + payload_schema: result: PASSED - title: This chapter should fail because the request body is invalid. overall: @@ -35,9 +37,11 @@ chapters: response: status: result: PASSED - payload: + payload_body: + result: PASSED + payload_schema: result: PASSED - - title: This chapter should fail because the response is invalid. + - title: This chapter should fail because the response data and schema are invalid. overall: result: FAILED request: @@ -49,7 +53,10 @@ chapters: response: status: result: PASSED - payload: + payload_body: + result: FAILED + message: expected acknowledged='false', got 'true', missing shards_acknowledged='true' + payload_schema: result: FAILED message: data must NOT have additional properties diff --git a/tools/tests/tester/fixtures/evals/failed/not_found.yaml b/tools/tests/tester/fixtures/evals/failed/not_found.yaml index 06d186c73..92ab01556 100644 --- a/tools/tests/tester/fixtures/evals/failed/not_found.yaml +++ b/tools/tests/tester/fixtures/evals/failed/not_found.yaml @@ -26,7 +26,9 @@ chapters: response: status: result: PASSED - payload: + payload_body: + result: PASSED + payload_schema: result: PASSED - title: This chapter should fail because the request body is not defined in the spec. overall: @@ -41,7 +43,9 @@ chapters: response: status: result: PASSED - payload: + payload_body: + result: PASSED + payload_schema: result: PASSED - title: This chapter should fail because the response is not defined in the spec. overall: @@ -55,7 +59,9 @@ chapters: response: status: result: PASSED - payload: + payload_body: + result: PASSED + payload_schema: result: FAILED message: 'Schema for "404: application/json" response not found in the spec.' diff --git a/tools/tests/tester/fixtures/evals/passed.yaml b/tools/tests/tester/fixtures/evals/passed.yaml index e7ec69ba3..bb0c28f5f 100644 --- a/tools/tests/tester/fixtures/evals/passed.yaml +++ b/tools/tests/tester/fixtures/evals/passed.yaml @@ -19,7 +19,9 @@ chapters: response: status: result: PASSED - payload: + payload_body: + result: PASSED + payload_schema: result: PASSED epilogues: diff --git a/tools/tests/tester/fixtures/stories/failed/invalid_data.yaml b/tools/tests/tester/fixtures/stories/failed/invalid_data.yaml index fbb6573b2..edac0cdd7 100644 --- a/tools/tests/tester/fixtures/stories/failed/invalid_data.yaml +++ b/tools/tests/tester/fixtures/stories/failed/invalid_data.yaml @@ -22,7 +22,7 @@ chapters: request_body: payload: aliases: {} - - synopsis: This chapter should fail because the response is invalid. + - synopsis: This chapter should fail because the response data and schema are invalid. path: /{index} method: DELETE parameters: @@ -30,4 +30,5 @@ chapters: response: status: 200 payload: - shards_acknowledged: true \ No newline at end of file + acknowledged: false + shards_acknowledged: true diff --git a/tools/tests/tester/helpers.ts b/tools/tests/tester/helpers.ts index 61c9a6070..65e9dd46e 100644 --- a/tools/tests/tester/helpers.ts +++ b/tools/tests/tester/helpers.ts @@ -90,7 +90,8 @@ export function flatten_errors (evaluation: StoryEvaluation): StoryEvaluation { response: c.response !== undefined ? { status: flatten(c.response.status), - payload: flatten(c.response.payload) + payload_body: flatten(c.response.payload_body), + payload_schema: flatten(c.response.payload_schema) } : undefined })) as T