From 0845299249f1aaf487538b760b5bc7ac2c29ed53 Mon Sep 17 00:00:00 2001 From: dblock Date: Tue, 27 Aug 2024 15:58:03 -0400 Subject: [PATCH] Fix: evaluate output values before schema checking. Signed-off-by: dblock --- tools/src/tester/ChapterEvaluator.ts | 14 ++-- .../fixtures/evals/{ => passed}/passed.yaml | 4 +- .../fixtures/evals/passed/value_type.yaml | 68 +++++++++++++++++++ .../tests/tester/fixtures/specs/excerpt.yaml | 40 +++++++---- .../fixtures/stories/{ => passed}/passed.yaml | 2 +- .../fixtures/stories/passed/value_type.yaml | 31 +++++++++ .../tests/tester/integ/StoryEvaluator.test.ts | 12 +++- tools/tests/tester/integ/TestRunner.test.ts | 5 +- 8 files changed, 148 insertions(+), 28 deletions(-) rename tools/tests/tester/fixtures/evals/{ => passed}/passed.yaml (98%) create mode 100644 tools/tests/tester/fixtures/evals/passed/value_type.yaml rename tools/tests/tester/fixtures/stories/{ => passed}/passed.yaml (97%) create mode 100644 tools/tests/tester/fixtures/stories/passed/value_type.yaml diff --git a/tools/src/tester/ChapterEvaluator.ts b/tools/src/tester/ChapterEvaluator.ts index 6feb7d0a0..04df8af42 100644 --- a/tools/src/tester/ChapterEvaluator.ts +++ b/tools/src/tester/ChapterEvaluator.ts @@ -63,8 +63,8 @@ export default class ChapterEvaluator { async #evaluate(chapter: Chapter, operation: ParsedOperation, story_outputs: StoryOutputs, retries?: number): Promise { const response = await this._chapter_reader.read(chapter, story_outputs) - const params = this.#evaluate_parameters(chapter, operation) - const request = this.#evaluate_request(chapter, operation) + const params = this.#evaluate_parameters(chapter, operation, story_outputs) + const request = this.#evaluate_request(chapter, operation, story_outputs) const status = this.#evaluate_status(chapter, response) const payload_schema_evaluation = status.result === Result.PASSED ? this.#evaluate_payload_schema(chapter, response, operation) : { result: Result.SKIPPED } const output_values_evaluation: EvaluationWithOutput = status.result === Result.PASSED ? ChapterOutput.extract_output_values(response, chapter.output) : { evaluation: { result: Result.SKIPPED } } @@ -110,8 +110,9 @@ export default class ChapterEvaluator { return result } - #evaluate_parameters(chapter: Chapter, operation: ParsedOperation): Record { - return Object.fromEntries(Object.entries(chapter.parameters ?? {}).map(([name, parameter]) => { + #evaluate_parameters(chapter: Chapter, operation: ParsedOperation, story_outputs: StoryOutputs): Record { + const parameters: Record = story_outputs.resolve_value(chapter.parameters) ?? {} + return Object.fromEntries(Object.entries(parameters).map(([name, parameter]) => { const schema = operation.parameters[name]?.schema if (schema == null) return [name, { result: Result.FAILED, message: `Schema for "${name}" parameter not found.` }] const evaluation = this._schema_validator.validate(schema, parameter) @@ -119,12 +120,13 @@ export default class ChapterEvaluator { })) } - #evaluate_request(chapter: Chapter, operation: ParsedOperation): Evaluation { + #evaluate_request(chapter: Chapter, operation: ParsedOperation, story_outputs: StoryOutputs): Evaluation { if (chapter.request?.payload === undefined) return { result: Result.PASSED } const content_type = chapter.request.content_type ?? APPLICATION_JSON const schema = operation.requestBody?.content[content_type]?.schema if (schema == null) return { result: Result.FAILED, message: `Schema for "${content_type}" request body not found in the spec.` } - return this._schema_validator.validate(schema, chapter.request?.payload ?? {}) + const payload = story_outputs.resolve_value(chapter.request?.payload) ?? {} + return this._schema_validator.validate(schema, payload) } #evaluate_status(chapter: Chapter, response: ActualResponse): Evaluation { diff --git a/tools/tests/tester/fixtures/evals/passed.yaml b/tools/tests/tester/fixtures/evals/passed/passed.yaml similarity index 98% rename from tools/tests/tester/fixtures/evals/passed.yaml rename to tools/tests/tester/fixtures/evals/passed/passed.yaml index e20d6fc4a..da08a29fa 100644 --- a/tools/tests/tester/fixtures/evals/passed.yaml +++ b/tools/tests/tester/fixtures/evals/passed/passed.yaml @@ -1,5 +1,5 @@ -display_path: passed.yaml -full_path: tools/tests/tester/fixtures/stories/passed.yaml +display_path: passed/passed.yaml +full_path: tools/tests/tester/fixtures/stories/passed/passed.yaml result: PASSED description: This story should pass. diff --git a/tools/tests/tester/fixtures/evals/passed/value_type.yaml b/tools/tests/tester/fixtures/evals/passed/value_type.yaml new file mode 100644 index 000000000..e2ad0269c --- /dev/null +++ b/tools/tests/tester/fixtures/evals/passed/value_type.yaml @@ -0,0 +1,68 @@ +display_path: passed/value_type.yaml +full_path: tools/tests/tester/fixtures/stories/passed/value_type.yaml + +result: PASSED +description: This story has an error trying to reuse a variable of a different type. +warnings: + - | + Multiple paths detected, please group similar tests together and move paths not being tested to prologues or epilogues. + /_cat/health + /{index} +prologues: + - overall: + result: PASSED + title: GET / + output: + outputs: + boolean: false +epilogues: + - overall: + result: PASSED + title: DELETE /movies +chapters: + - title: This chapter requires a boolean in the parameters. + overall: + result: PASSED + path: GET /_cat/health + operation: + method: GET + path: /_cat/health + request: + parameters: + format: + result: PASSED + v: + result: PASSED + request: + result: PASSED + response: + status: + result: PASSED + payload_body: + result: PASSED + payload_schema: + result: PASSED + output_values: + result: SKIPPED + - title: This chapter requires a boolean in the body. + overall: + result: PASSED + path: PUT /{index} + operation: + method: PUT + path: /{index} + request: + parameters: + index: + result: PASSED + request: + result: PASSED + response: + status: + result: PASSED + payload_body: + result: PASSED + payload_schema: + result: PASSED + output_values: + result: SKIPPED \ No newline at end of file diff --git a/tools/tests/tester/fixtures/specs/excerpt.yaml b/tools/tests/tester/fixtures/specs/excerpt.yaml index 244e7e466..5ff3c005e 100644 --- a/tools/tests/tester/fixtures/specs/excerpt.yaml +++ b/tools/tests/tester/fixtures/specs/excerpt.yaml @@ -22,6 +22,7 @@ paths: description: Returns health for the Cat APIs. parameters: - $ref: '#/components/parameters/cat.health::query.format' + - $ref: '#/components/parameters/cat.health::query.v' responses: '200': $ref: '#/components/responses/cat.health@200' @@ -40,6 +41,20 @@ paths: '200': $ref: '#/components/responses/cat.indices@200' /{index}: + put: + operationId: indices.create.0 + x-operation-group: indices.create + x-version-added: '1.0' + description: Creates an index with optional settings and mappings. + externalDocs: + url: https://opensearch.org/docs/latest/api-reference/index-apis/create-index/ + parameters: + - $ref: '#/components/parameters/indices.create::path.index' + requestBody: + $ref: '#/components/requestBodies/indices.create' + responses: + '200': + $ref: '#/components/responses/indices.create@200' delete: operationId: indices.delete.0 x-operation-group: indices.delete @@ -65,20 +80,6 @@ paths: responses: '200': $ref: '#/components/responses/indices.exists@200' - put: - operationId: indices.create.0 - x-operation-group: indices.create - x-version-added: '1.0' - description: Creates an index with optional settings and mappings. - externalDocs: - url: https://opensearch.org/docs/latest/api-reference/index-apis/create-index/ - parameters: - - $ref: '#/components/parameters/indices.create::path.index' - requestBody: - $ref: '#/components/requestBodies/indices.create' - responses: - '200': - $ref: '#/components/responses/indices.create@200' components: requestBodies: indices.create: @@ -90,6 +91,12 @@ components: settings: type: object description: Optional settings for the index. + properties: + shard: + type: object + properties: + check_on_startup: + type: boolean mappings: type: object description: Optional mappings for the index. @@ -180,6 +187,11 @@ components: name: format schema: type: string + cat.health::query.v: + in: query + name: v + schema: + type: boolean cat.help::query.format: in: query name: format diff --git a/tools/tests/tester/fixtures/stories/passed.yaml b/tools/tests/tester/fixtures/stories/passed/passed.yaml similarity index 97% rename from tools/tests/tester/fixtures/stories/passed.yaml rename to tools/tests/tester/fixtures/stories/passed/passed.yaml index 7c51b9dc0..3181f9ede 100644 --- a/tools/tests/tester/fixtures/stories/passed.yaml +++ b/tools/tests/tester/fixtures/stories/passed/passed.yaml @@ -1,4 +1,4 @@ -$schema: ../../../../../json_schemas/test_story.schema.yaml +$schema: ../../../../../../json_schemas/test_story.schema.yaml description: This story should pass. warnings: diff --git a/tools/tests/tester/fixtures/stories/passed/value_type.yaml b/tools/tests/tester/fixtures/stories/passed/value_type.yaml new file mode 100644 index 000000000..ffc7b6b85 --- /dev/null +++ b/tools/tests/tester/fixtures/stories/passed/value_type.yaml @@ -0,0 +1,31 @@ +$schema: ../../../../../../json_schemas/test_story.schema.yaml + +description: This story has an error trying to reuse a variable of a different type. + +prologues: + - method: GET + id: root + path: / + output: + boolean: payload.version.build_snapshot +epilogues: + - method: DELETE + path: /movies + status: [200, 404] +chapters: + - synopsis: This chapter requires a boolean in the parameters. + method: GET + path: /_cat/health + parameters: + format: json + v: ${root.boolean} + - synopsis: This chapter requires a boolean in the body. + method: PUT + path: /{index} + parameters: + index: movies + request: + payload: + settings: + shard: + check_on_startup: ${root.boolean} diff --git a/tools/tests/tester/integ/StoryEvaluator.test.ts b/tools/tests/tester/integ/StoryEvaluator.test.ts index f3c6bc15e..cae1de746 100644 --- a/tools/tests/tester/integ/StoryEvaluator.test.ts +++ b/tools/tests/tester/integ/StoryEvaluator.test.ts @@ -22,8 +22,14 @@ afterEach(() => { }) test('passed', async () => { - const actual = await load_actual_evaluation(story_evaluator, 'passed') - const expected = load_expected_evaluation('passed') + const actual = await load_actual_evaluation(story_evaluator, 'passed/passed') + const expected = load_expected_evaluation('passed/passed') + expect(actual).toEqual(expected) +}) + +test('value_type.yaml', async () => { + const actual = await load_actual_evaluation(story_evaluator, 'passed/value_type') + const expected = load_expected_evaluation('passed/value_type') expect(actual).toEqual(expected) }) @@ -65,7 +71,7 @@ test('skipped/semver', async () => { test('with an unexpected error deserializing data', async () => { opensearch_http_client.request = jest.fn().mockRejectedValue(new Error('This was unexpected.')) - const actual = await load_actual_evaluation(story_evaluator, 'passed') + const actual = await load_actual_evaluation(story_evaluator, 'passed/passed') expect(actual.result).toEqual(Result.ERROR) expect(actual.chapters && actual.chapters[0]).toEqual({ title: "This PUT /{index} chapter should pass.", diff --git a/tools/tests/tester/integ/TestRunner.test.ts b/tools/tests/tester/integ/TestRunner.test.ts index c1b3e2196..75253cd7c 100644 --- a/tools/tests/tester/integ/TestRunner.test.ts +++ b/tools/tests/tester/integ/TestRunner.test.ts @@ -29,19 +29,20 @@ test('stories folder', async () => { } const expected_evaluations = [ - 'passed', 'error/chapter_error', 'error/output_error', 'error/prologue_error', 'failed/invalid_data', 'failed/not_found', + 'passed/passed', + 'passed/value_type', 'skipped/semver', 'skipped/distributions/chapters', 'skipped/distributions/excluded', 'skipped/distributions/included' ].map((fixture) => { return load_expected_evaluation(fixture, true) }) - expect(actual_evaluations).toStrictEqual(expected_evaluations) + expect(actual_evaluations).toEqual(expected_evaluations) }) describe('story_files', () => {