From b9388d46f9a30b7c26f16daa6d0a6899cbf55786 Mon Sep 17 00:00:00 2001 From: "Daniel (dB.) Doubrovkine" Date: Thu, 25 Jul 2024 11:18:25 -0500 Subject: [PATCH] Fix: display output collection errors. (#450) * Fix: display output collection errors. Signed-off-by: dblock --- tools/src/tester/ChapterEvaluator.ts | 32 ++++++++++---- tools/src/tester/ChapterOutput.ts | 12 +++--- tools/src/tester/ResultLogger.ts | 6 +++ tools/src/tester/StoryEvaluator.ts | 10 ++--- .../tester/SupplementalChapterEvaluator.ts | 42 ++++++++++++++----- tools/src/tester/types/eval.types.ts | 8 ++-- .../fixtures/evals/error/chapter_error.yaml | 2 + .../fixtures/evals/error/output_error.yaml | 29 +++++++++++++ .../fixtures/evals/failed/invalid_data.yaml | 10 +++++ .../fixtures/evals/failed/not_found.yaml | 6 +++ tools/tests/tester/fixtures/evals/passed.yaml | 16 +++++++ .../fixtures/stories/error/output_error.yaml | 15 +++++++ tools/tests/tester/helpers.ts | 3 +- tools/tests/tester/integ/TestRunner.test.ts | 3 +- tools/tests/tester/test.test.ts | 10 +++-- 15 files changed, 165 insertions(+), 39 deletions(-) create mode 100644 tools/tests/tester/fixtures/evals/error/output_error.yaml create mode 100644 tools/tests/tester/fixtures/stories/error/output_error.yaml diff --git a/tools/src/tester/ChapterEvaluator.ts b/tools/src/tester/ChapterEvaluator.ts index a75b2094..55378363 100644 --- a/tools/src/tester/ChapterEvaluator.ts +++ b/tools/src/tester/ChapterEvaluator.ts @@ -8,7 +8,7 @@ */ import { type Chapter, type ActualResponse, type Payload } from './types/story.types' -import { type ChapterEvaluation, type Evaluation, Result } from './types/eval.types' +import { type ChapterEvaluation, type Evaluation, Result, EvaluationWithOutput } from './types/eval.types' import { type ParsedOperation } from './types/spec.types' import { overall_result } from './helpers' import type ChapterReader from './ChapterReader' @@ -45,21 +45,35 @@ export default class ChapterEvaluator { const status = this.#evaluate_status(chapter, response) 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(chapter, response, operation) : { result: Result.SKIPPED } - const output_values = ChapterOutput.extract_output_values(response, chapter.output) - return { + const output_values_evaluation: EvaluationWithOutput = status.result === Result.PASSED ? ChapterOutput.extract_output_values(response, chapter.output) : { evaluation: { result: Result.SKIPPED } } + + const evaluations = _.compact(_.concat( + Object.values(params), + request_body, + status, + payload_body_evaluation, + payload_schema_evaluation, + output_values_evaluation.evaluation + )) + + var result: ChapterEvaluation = { title: chapter.synopsis, path: `${chapter.method} ${chapter.path}`, - overall: { result: overall_result(Object.values(params).concat([ - request_body, status, payload_body_evaluation, payload_schema_evaluation - ]).concat(output_values ? [output_values] : [])) }, + overall: { result: overall_result(evaluations) }, request: { parameters: params, request_body }, response: { status, payload_body: payload_body_evaluation, - payload_schema: payload_schema_evaluation - }, - ...(output_values ? { output_values } : {}) + payload_schema: payload_schema_evaluation, + output_values: output_values_evaluation.evaluation + } + } + + if (output_values_evaluation?.output !== undefined) { + result.output = output_values_evaluation?.output } + + return result } #evaluate_parameters(chapter: Chapter, operation: ParsedOperation): Record { diff --git a/tools/src/tester/ChapterOutput.ts b/tools/src/tester/ChapterOutput.ts index 7ad1def9..5a8e07b2 100644 --- a/tools/src/tester/ChapterOutput.ts +++ b/tools/src/tester/ChapterOutput.ts @@ -26,8 +26,8 @@ export class ChapterOutput { this.outputs[name] = value } - static extract_output_values(response: ActualResponse, output?: Output): EvaluationWithOutput | undefined { - if (!output) return undefined + static extract_output_values(response: ActualResponse, output?: Output): EvaluationWithOutput { + if (!output) return { evaluation: { result: Result.SKIPPED } } const chapter_output = new ChapterOutput({}) for (const [name, path] of Object.entries(output)) { const [source, ...rest] = path.split('.') @@ -35,17 +35,17 @@ export class ChapterOutput { let value: any if (source === 'payload') { if (response.payload === undefined) { - return { result: Result.ERROR, message: 'No payload found in response, but expected output: ' + path } + return { evaluation: { result: Result.ERROR, message: 'No payload found in response, but expected output: ' + path } } } value = keys.length === 0 ? response.payload : _.get(response.payload, keys) if (value === undefined) { - return { result: Result.ERROR, message: `Expected to find non undefined value at \`${path}\`.` } + return { evaluation: { result: Result.ERROR, message: `Expected to find non undefined value at \`${path}\`.` } } } } else { - return { result: Result.ERROR, message: 'Unknown output source: ' + source } + return { evaluation: { result: Result.ERROR, message: 'Unknown output source: ' + source } } } chapter_output.set(name, value) } - return { result: Result.PASSED, output: chapter_output } + return { evaluation: { result: Result.PASSED }, output: chapter_output } } } diff --git a/tools/src/tester/ResultLogger.ts b/tools/src/tester/ResultLogger.ts index aa151849..f660ac8c 100644 --- a/tools/src/tester/ResultLogger.ts +++ b/tools/src/tester/ResultLogger.ts @@ -65,6 +65,7 @@ export class ConsoleResultLogger implements ResultLogger { this.#log_status(chapter.response?.status) this.#log_payload_body(chapter.response?.payload_body) this.#log_payload_schema(chapter.response?.payload_schema) + this.#log_output_values(chapter.response?.output_values) } #log_parameters (parameters: Record): void { @@ -96,6 +97,11 @@ export class ConsoleResultLogger implements ResultLogger { this.#log_evaluation(evaluation, 'RESPONSE PAYLOAD SCHEMA', this._tab_width * 3) } + #log_output_values (evaluation: Evaluation | undefined): void { + if (evaluation == null) return + this.#log_evaluation(evaluation, 'RESPONSE OUTPUT VALUES', this._tab_width * 3) + } + #log_evaluation (evaluation: Evaluation, title: string, prefix: number = 0): void { const result = ansi.padding(this.#result(evaluation.result), 0, prefix) diff --git a/tools/src/tester/StoryEvaluator.ts b/tools/src/tester/StoryEvaluator.ts index 719437dc..e4da7b16 100644 --- a/tools/src/tester/StoryEvaluator.ts +++ b/tools/src/tester/StoryEvaluator.ts @@ -67,8 +67,8 @@ export default class StoryEvaluator { } else { const evaluation = await this._chapter_evaluator.evaluate(chapter, has_errors, story_outputs) has_errors = has_errors || evaluation.overall.result === Result.ERROR - if (evaluation.output_values?.output !== undefined && chapter.id !== undefined) { - story_outputs.set_chapter_output(chapter.id, evaluation.output_values?.output) + if (evaluation.output !== undefined && chapter.id !== undefined) { + story_outputs.set_chapter_output(chapter.id, evaluation.output) } evaluations.push(evaluation) } @@ -86,8 +86,8 @@ export default class StoryEvaluator { } else { const { evaluation, evaluation_error } = await this._supplemental_chapter_evaluator.evaluate(chapter, story_outputs) has_errors = has_errors || evaluation_error - if (evaluation.output_values?.output !== undefined && chapter.id !== undefined) { - story_outputs.set_chapter_output(chapter.id, evaluation.output_values?.output) + if (evaluation.output !== undefined && chapter.id !== undefined) { + story_outputs.set_chapter_output(chapter.id, evaluation.output) } evaluations.push(evaluation) } @@ -117,7 +117,7 @@ export default class StoryEvaluator { prologues, chapters, epilogues, - message: 'The story was defined with incorrect variables' + message: 'The story was defined with incorrect variables.' } } } diff --git a/tools/src/tester/SupplementalChapterEvaluator.ts b/tools/src/tester/SupplementalChapterEvaluator.ts index 41961be0..81b680be 100644 --- a/tools/src/tester/SupplementalChapterEvaluator.ts +++ b/tools/src/tester/SupplementalChapterEvaluator.ts @@ -7,6 +7,7 @@ * compatible open source license. */ +import _ from "lodash"; import { ChapterOutput } from "./ChapterOutput"; import ChapterReader from "./ChapterReader"; import { StoryOutputs } from "./StoryOutputs"; @@ -25,34 +26,55 @@ export default class SupplementalChapterEvaluator { const title = `${chapter.method} ${chapter.path}` const response = await this._chapter_reader.read(chapter, story_outputs) const status = chapter.status ?? [200, 201] - const output_values = ChapterOutput.extract_output_values(response, chapter.output) + const output_values_evaluation = ChapterOutput.extract_output_values(response, chapter.output) let response_evaluation: ChapterEvaluation const passed_evaluation = { title, overall: { result: Result.PASSED } } if (status.includes(response.status)) { response_evaluation = passed_evaluation } else { - response_evaluation = { title, overall: { result: Result.ERROR, message: response.message, error: response.error as Error }, output_values } + response_evaluation = { + title, + overall: { + result: Result.ERROR, + message: response.message, + error: response.error as Error + } + } } - if (output_values) { - response_evaluation.output_values = output_values + + if (output_values_evaluation.output) { + response_evaluation.output = output_values_evaluation.output } - const result = overall_result([response_evaluation.overall].concat(output_values ? [output_values] : [])) + + const result = overall_result(_.compact([ + response_evaluation.overall, + output_values_evaluation.evaluation + ])) + if (result === Result.PASSED) { return { evaluation: passed_evaluation, evaluation_error: false } } else { const message_segments = [] + if (response_evaluation.overall.result === Result.ERROR) { message_segments.push(`${response_evaluation.overall.message}`) } - if (output_values !== undefined && output_values.result === Result.ERROR) { - message_segments.push(`${output_values.message}`) + + if (output_values_evaluation.evaluation.message !== undefined && output_values_evaluation.evaluation.result === Result.ERROR) { + message_segments.push(`${output_values_evaluation.evaluation.message}`) } + const message = message_segments.join('\n') - const evaluation = { + + var evaluation: ChapterEvaluation = { title, - overall: { result: Result.ERROR, message, error: response.error as Error }, - ...(output_values ? { output_values } : {}) + overall: { result: Result.ERROR, message, error: response.error as Error } } + + if (output_values_evaluation.output) { + evaluation.output = output_values_evaluation.output + } + return { evaluation, evaluation_error: true } } } diff --git a/tools/src/tester/types/eval.types.ts b/tools/src/tester/types/eval.types.ts index 0097361e..2f099b68 100644 --- a/tools/src/tester/types/eval.types.ts +++ b/tools/src/tester/types/eval.types.ts @@ -44,8 +44,9 @@ export interface ChapterEvaluation { status: Evaluation payload_body: Evaluation, payload_schema: Evaluation - } - output_values?: EvaluationWithOutput + output_values: Evaluation + }, + output?: ChapterOutput } export class ChaptersEvaluations { @@ -63,7 +64,8 @@ export interface Evaluation { error?: Error | string } -export type EvaluationWithOutput = Evaluation & { +export type EvaluationWithOutput = { + evaluation: Evaluation, output?: ChapterOutput } diff --git a/tools/tests/tester/fixtures/evals/error/chapter_error.yaml b/tools/tests/tester/fixtures/evals/error/chapter_error.yaml index 37f1baba..9ea8e1c7 100644 --- a/tools/tests/tester/fixtures/evals/error/chapter_error.yaml +++ b/tools/tests/tester/fixtures/evals/error/chapter_error.yaml @@ -32,6 +32,8 @@ chapters: result: SKIPPED payload_schema: result: SKIPPED + output_values: + result: SKIPPED - title: This chapter should be skipped. overall: result: SKIPPED diff --git a/tools/tests/tester/fixtures/evals/error/output_error.yaml b/tools/tests/tester/fixtures/evals/error/output_error.yaml new file mode 100644 index 00000000..4ff037a0 --- /dev/null +++ b/tools/tests/tester/fixtures/evals/error/output_error.yaml @@ -0,0 +1,29 @@ +display_path: error/output_error.yaml +full_path: tools/tests/tester/fixtures/stories/error/output_error.yaml + +result: ERROR +description: This story has an error in the output. +prologues: [] +epilogues: [] +chapters: + - title: This chapter expects a `cursor` in the output. + overall: + result: ERROR + path: GET /_cat/health + request: + parameters: + format: + result: PASSED + request_body: + result: PASSED + response: + payload_body: + result: PASSED + payload_schema: + result: PASSED + output_values: + result: ERROR + message: Expected to find non undefined value at `payload.does_not_exist`. + status: + result: PASSED + diff --git a/tools/tests/tester/fixtures/evals/failed/invalid_data.yaml b/tools/tests/tester/fixtures/evals/failed/invalid_data.yaml index a05ae7aa..10782489 100644 --- a/tools/tests/tester/fixtures/evals/failed/invalid_data.yaml +++ b/tools/tests/tester/fixtures/evals/failed/invalid_data.yaml @@ -25,6 +25,8 @@ chapters: result: PASSED payload_schema: result: PASSED + output_values: + result: SKIPPED - title: This chapter should fail because the request body is invalid. overall: result: FAILED @@ -43,6 +45,8 @@ chapters: result: PASSED payload_schema: result: PASSED + output_values: + result: SKIPPED - title: This chapter should fail because the response content type does not match. overall: result: FAILED @@ -63,6 +67,8 @@ chapters: payload_schema: result: FAILED message: 'Expected content type application/json, but received application/yaml.' + output_values: + result: SKIPPED - title: This chapter should fail because the response data and schema are invalid. overall: result: FAILED @@ -82,6 +88,8 @@ chapters: payload_schema: result: FAILED message: 'data contains unsupported properties: acknowledged' + output_values: + result: SKIPPED - title: This chapter should fail because the response status does not match. overall: result: ERROR @@ -100,6 +108,8 @@ chapters: result: SKIPPED payload_schema: result: SKIPPED + output_values: + result: SKIPPED epilogues: - title: DELETE /books diff --git a/tools/tests/tester/fixtures/evals/failed/not_found.yaml b/tools/tests/tester/fixtures/evals/failed/not_found.yaml index 72bc6318..378cb068 100644 --- a/tools/tests/tester/fixtures/evals/failed/not_found.yaml +++ b/tools/tests/tester/fixtures/evals/failed/not_found.yaml @@ -31,6 +31,8 @@ chapters: result: PASSED payload_schema: result: PASSED + output_values: + result: SKIPPED - title: This chapter should fail because the request body is not defined in the spec. overall: result: FAILED @@ -49,6 +51,8 @@ chapters: result: PASSED payload_schema: result: PASSED + output_values: + result: SKIPPED - title: This chapter should fail because the response is not defined in the spec. overall: result: FAILED @@ -67,6 +71,8 @@ chapters: payload_schema: result: FAILED message: 'Schema for "404: application/json" response not found in the spec.' + output_values: + result: SKIPPED epilogues: - title: DELETE /books diff --git a/tools/tests/tester/fixtures/evals/passed.yaml b/tools/tests/tester/fixtures/evals/passed.yaml index 375b24f2..865df7b0 100644 --- a/tools/tests/tester/fixtures/evals/passed.yaml +++ b/tools/tests/tester/fixtures/evals/passed.yaml @@ -24,6 +24,8 @@ chapters: result: PASSED payload_schema: result: PASSED + output_values: + result: SKIPPED - title: This GET /_cat chapter returns text/plain and should pass. overall: result: PASSED @@ -39,6 +41,8 @@ chapters: result: PASSED payload_schema: result: PASSED + output_values: + result: SKIPPED - title: This GET /_cat/health chapter returns application/json and should pass. overall: result: PASSED @@ -56,6 +60,8 @@ chapters: result: PASSED payload_schema: result: PASSED + output_values: + result: SKIPPED - title: This GET /_cat/health chapter returns application/yaml and should pass. overall: result: PASSED @@ -73,6 +79,8 @@ chapters: result: PASSED payload_schema: result: PASSED + output_values: + result: SKIPPED - title: This GET /_cat/health chapter returns application/cbor and should pass. overall: result: PASSED @@ -90,6 +98,8 @@ chapters: result: PASSED payload_schema: result: PASSED + output_values: + result: SKIPPED - title: This GET /_cat/health chapter returns application/smile and should pass. overall: result: PASSED @@ -107,6 +117,8 @@ chapters: result: PASSED payload_schema: result: PASSED + output_values: + result: SKIPPED - title: This GET /_cat/health should run (default). overall: result: PASSED @@ -124,6 +136,8 @@ chapters: result: PASSED payload_schema: result: PASSED + output_values: + result: SKIPPED - title: This GET /_cat/health should run (~> 2.x). overall: result: PASSED @@ -141,6 +155,8 @@ chapters: result: PASSED payload_schema: result: PASSED + output_values: + result: SKIPPED - title: This GET /_cat/health should be skipped (> 2.999.0). overall: result: SKIPPED diff --git a/tools/tests/tester/fixtures/stories/error/output_error.yaml b/tools/tests/tester/fixtures/stories/error/output_error.yaml new file mode 100644 index 00000000..e5c72520 --- /dev/null +++ b/tools/tests/tester/fixtures/stories/error/output_error.yaml @@ -0,0 +1,15 @@ +$schema: ../../../../../../json_schemas/test_story.schema.yaml + +description: This story has an error in the output. + +chapters: + - synopsis: This chapter expects a `cursor` in the output. + method: GET + path: /_cat/health + parameters: + format: json + id: cat.health + response: + status: 200 + output: + cursor: payload.does_not_exist diff --git a/tools/tests/tester/helpers.ts b/tools/tests/tester/helpers.ts index 73c07a93..57cbcff1 100644 --- a/tools/tests/tester/helpers.ts +++ b/tools/tests/tester/helpers.ts @@ -111,7 +111,8 @@ export function flatten_errors (evaluation: StoryEvaluation): StoryEvaluation { result.response = { status: flatten(c.response.status), payload_body: flatten(c.response.payload_body), - payload_schema: flatten(c.response.payload_schema) + payload_schema: flatten(c.response.payload_schema), + output_values: flatten(c.response.output_values) } } diff --git a/tools/tests/tester/integ/TestRunner.test.ts b/tools/tests/tester/integ/TestRunner.test.ts index 4e06a109..99d1e1ba 100644 --- a/tools/tests/tester/integ/TestRunner.test.ts +++ b/tools/tests/tester/integ/TestRunner.test.ts @@ -33,8 +33,9 @@ test('stories folder', async () => { const not_found = load_expected_evaluation('failed/not_found', true) const invalid_data = load_expected_evaluation('failed/invalid_data', true) const chapter_error = load_expected_evaluation('error/chapter_error', true) + const output_error = load_expected_evaluation('error/output_error', true) const prologue_error = load_expected_evaluation('error/prologue_error', true) - const expected_evaluations = [passed, chapter_error, prologue_error, invalid_data, not_found, skipped] + const expected_evaluations = [passed, chapter_error, output_error, prologue_error, invalid_data, not_found, skipped] expect(actual_evaluations).toEqual(expected_evaluations) }) diff --git a/tools/tests/tester/test.test.ts b/tools/tests/tester/test.test.ts index 06dbd2de..a7db537d 100644 --- a/tools/tests/tester/test.test.ts +++ b/tools/tests/tester/test.test.ts @@ -55,7 +55,7 @@ function create_response(payload: any): ActualResponse { function passed_output(output: Record): EvaluationWithOutput { return { - result: Result.PASSED, + evaluation: { result: Result.PASSED }, output: new ChapterOutput(output) } } @@ -86,8 +86,10 @@ test('extract_output_values', () => { passed_output({ x: response.payload }) ) expect(ChapterOutput.extract_output_values(response, { x: 'payload.a.b.x[0]' })).toEqual({ - result: Result.ERROR, - message: 'Expected to find non undefined value at `payload.a.b.x[0]`.' + evaluation: { + result: Result.ERROR, + message: 'Expected to find non undefined value at `payload.a.b.x[0]`.' + } }) }) @@ -123,7 +125,7 @@ test('check_story_variables', () => { description: 'story1', display_path: 'display_path', full_path: 'full_path', - message: 'The story was defined with incorrect variables', + message: 'The story was defined with incorrect variables.', prologues, chapters, epilogues: []