From b67104c30e515702df88bba9c173d04033d07a45 Mon Sep 17 00:00:00 2001 From: "Daniel (dB.) Doubrovkine" Date: Thu, 1 Aug 2024 17:07:17 -0400 Subject: [PATCH] Output a warning on mulitple paths being tested in the same file. (#452) * Output a warning on mulitple paths being tested in the same file. Signed-off-by: dblock * Support disabling warnings. Signed-off-by: dblock * Fixed prologue warnings. Signed-off-by: dblock --------- Signed-off-by: dblock --- CHANGELOG.md | 3 +- TESTING_GUIDE.md | 34 ++++++++++ json_schemas/test_story.schema.yaml | 11 +++ tests/_core/reindex/pipeline.yaml | 4 ++ tests/cat/pit_segments/all.yaml | 36 ++++++++++ .../cat/{ => pit_segments}/pit_segments.yaml | 16 +---- tests/cat/snapshots.yaml | 12 ++-- tests/indices/alias/alias.yaml | 68 +++++++++++++++++++ tests/indices/aliases/aliases.yaml | 67 +----------------- tests/indices/clone.yaml | 15 ++-- tests/indices/component_template.yaml | 8 +++ .../{ => data_stream}/data_stream.yaml | 14 +--- tests/indices/data_stream/rollover.yaml | 38 +++++++++++ tests/indices/data_stream/stats.yaml | 36 ++++++++++ tests/indices/index.yaml | 12 ---- tests/indices/shrink.yaml | 15 ++-- tests/indices/split.yaml | 15 ++-- tests/ml/model_groups.yaml | 20 +++--- tests/ml/models.yaml | 2 + tests/nodes/stats.yaml | 2 + tests/sql/close.yaml | 15 ++-- tools/src/tester/ResultLogger.ts | 8 ++- tools/src/tester/StoryEvaluator.ts | 23 ++++++- tools/src/tester/types/eval.types.ts | 1 + tools/src/tester/types/story.types.ts | 8 +++ tools/tests/tester/ResultLogger.test.ts | 25 +++++++ .../fixtures/evals/error/chapter_error.yaml | 6 ++ .../fixtures/evals/error/output_error.yaml | 1 + .../fixtures/evals/error/prologue_error.yaml | 6 +- .../fixtures/evals/failed/invalid_data.yaml | 2 - .../fixtures/evals/failed/not_found.yaml | 7 +- tools/tests/tester/fixtures/evals/passed.yaml | 2 - .../tester/fixtures/evals/skipped/semver.yaml | 1 + .../fixtures/stories/failed/invalid_data.yaml | 2 + .../tests/tester/fixtures/stories/passed.yaml | 4 ++ .../tests/tester/integ/StoryEvaluator.test.ts | 11 +++ 36 files changed, 386 insertions(+), 164 deletions(-) create mode 100644 tests/cat/pit_segments/all.yaml rename tests/cat/{ => pit_segments}/pit_segments.yaml (71%) create mode 100644 tests/indices/alias/alias.yaml rename tests/indices/{ => data_stream}/data_stream.yaml (79%) create mode 100644 tests/indices/data_stream/rollover.yaml create mode 100644 tests/indices/data_stream/stats.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index 293500bca..453d4697a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,7 +57,8 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Added `--opensearch-version` to `merger` that excludes schema elements per semver ([#428](https://github.com/opensearch-project/opensearch-api-specification/pull/428)) - Added `retry` to `tester` to support asynchronous tasks ([453](https://github.com/opensearch-project/opensearch-api-specification/pull/453)) - Added passing OPENSEARCH_JAVA_OPTS into the docker container used for tests ([#454](https://github.com/opensearch-project/opensearch-api-specification/pull/454)) - +- Added a warning on mulitple paths being tested in the same file ([#452](https://github.com/opensearch-project/opensearch-api-specification/pull/452)) + ### Changed - Replaced Smithy with a native OpenAPI spec ([#189](https://github.com/opensearch-project/opensearch-api-specification/issues/189)) diff --git a/TESTING_GUIDE.md b/TESTING_GUIDE.md index e4e34d71b..efe86d41e 100644 --- a/TESTING_GUIDE.md +++ b/TESTING_GUIDE.md @@ -6,6 +6,9 @@ - [Using Output from Previous Chapters](#using-output-from-previous-chapters) - [Managing Versions](#managing-versions) - [Waiting for Tasks](#waiting-for-tasks) + - [Warnings](#warnings) + - [multiple-paths-detected](#multiple-paths-detected) + - [Suppressing Warnings](#suppressing-warnings) # Spec Testing Guide @@ -175,3 +178,34 @@ For example, an ML task returns `CREATED` when created, and `COMPLETED` when it' count: 3 wait: 30000 ``` +### Warnings + +#### multiple-paths-detected + +The test runner expects all tests in the same file to be variation of the same path in order to keep tests well-organized. Otherwise, a warning will be emitted. + +``` +WARNING Multiple paths detected, please group similar tests together and move paths not being tested to prologues or epilogues. + /_component_template/{name} + /_index_template/{name} + /{index} +``` + +#### Suppressing Warnings + +The test runner may generate warnings that can be suppressed with `warnings:`. For example, to suppress the multiple paths detected warning. + +```yaml +- synopsis: Create an index. + method: PUT + path: /{index} + parameters: + index: movies +- synopsis: Search the index to make sure it has been created. + method: POST + warnings: + multiple-paths-detected: false + path: /{index}/_search + parameters: + index: movies +``` diff --git a/json_schemas/test_story.schema.yaml b/json_schemas/test_story.schema.yaml index 70defe2e9..74d16fd48 100644 --- a/json_schemas/test_story.schema.yaml +++ b/json_schemas/test_story.schema.yaml @@ -34,6 +34,8 @@ definitions: description: A brief description of the chapter. response: $ref: '#/definitions/ExpectedResponse' + warnings: + $ref: '#/definitions/Warnings' required: [synopsis] ReadChapter: @@ -180,3 +182,12 @@ definitions: - type: string - type: number - type: boolean + + Warnings: + type: object + properties: + multiple-paths-detected: + type: boolean + default: true + description: Enable/disable warnings about multiple paths being tested in the same story. + additionalProperties: false diff --git a/tests/_core/reindex/pipeline.yaml b/tests/_core/reindex/pipeline.yaml index aff582e48..f8e263ae7 100644 --- a/tests/_core/reindex/pipeline.yaml +++ b/tests/_core/reindex/pipeline.yaml @@ -54,11 +54,15 @@ chapters: response: status: 200 - synopsis: Refresh the index. + warnings: + multiple-paths-detected: false path: /{index}/_refresh method: POST parameters: index: videos - synopsis: Get all videos. + warnings: + multiple-paths-detected: false path: /{index}/_search method: POST parameters: diff --git a/tests/cat/pit_segments/all.yaml b/tests/cat/pit_segments/all.yaml new file mode 100644 index 000000000..5b8a1f9ff --- /dev/null +++ b/tests/cat/pit_segments/all.yaml @@ -0,0 +1,36 @@ +$schema: ../../../json_schemas/test_story.schema.yaml + +description: Test cat/pit_segments/_all endpoints. +version: '>= 2.4' +epilogues: + - path: /games + method: DELETE + status: [200, 404] +prologues: + - path: /games/_doc + method: POST + parameters: + refresh: true + request_body: + payload: + title: Monopoly + status: [201] + - id: create_pit + path: /{index}/_search/point_in_time + method: POST + parameters: + index: + - games + keep_alive: 1m + output: + pit_id: payload.pit_id +chapters: + - synopsis: Cat _all with a json response. + path: /_cat/pit_segments/_all + method: GET + parameters: + format: json + response: + status: 200 + payload: + - index: games diff --git a/tests/cat/pit_segments.yaml b/tests/cat/pit_segments/pit_segments.yaml similarity index 71% rename from tests/cat/pit_segments.yaml rename to tests/cat/pit_segments/pit_segments.yaml index 7bd02a6fe..172988830 100644 --- a/tests/cat/pit_segments.yaml +++ b/tests/cat/pit_segments/pit_segments.yaml @@ -1,4 +1,4 @@ -$schema: ../../json_schemas/test_story.schema.yaml +$schema: ../../../json_schemas/test_story.schema.yaml description: Test cat/pit_segments endpoints. version: '>= 2.4' @@ -15,9 +15,7 @@ prologues: payload: title: Monopoly status: [201] -chapters: - - synopsis: Create a PIT. - id: create_pit + - id: create_pit path: /{index}/_search/point_in_time method: POST parameters: @@ -26,15 +24,7 @@ chapters: keep_alive: 1m output: pit_id: payload.pit_id - - synopsis: Cat _all with a json response. - path: /_cat/pit_segments/_all - method: GET - parameters: - format: json - response: - status: 200 - payload: - - index: games +chapters: - synopsis: Cat pit_segments/pit_id with a json response. path: /_cat/pit_segments method: GET diff --git a/tests/cat/snapshots.yaml b/tests/cat/snapshots.yaml index 6a07ac3ce..1163c5db5 100644 --- a/tests/cat/snapshots.yaml +++ b/tests/cat/snapshots.yaml @@ -2,22 +2,18 @@ $schema: ../../json_schemas/test_story.schema.yaml description: Test cat/snapshots endpoints. epilogues: - - path: /_snapshot/{repository} + - path: /_snapshot/my-fs-repository method: DELETE status: [200, 404] - parameters: - repository: my-fs-repository -chapters: - - synopsis: Create a snapshot repository. - path: /_snapshot/{repository} +prologues: + - path: /_snapshot/my-fs-repository method: PUT - parameters: - repository: my-fs-repository request_body: payload: type: fs settings: location: /tmp/opensearch/repo +chapters: - synopsis: Cat with a json response. path: /_cat/snapshots/{repository} method: GET diff --git a/tests/indices/alias/alias.yaml b/tests/indices/alias/alias.yaml new file mode 100644 index 000000000..5bd41501c --- /dev/null +++ b/tests/indices/alias/alias.yaml @@ -0,0 +1,68 @@ +$schema: ../../../json_schemas/test_story.schema.yaml + +description: Test alias endpoints. +epilogues: + - path: /games + method: DELETE + status: [200, 404] +prologues: + - path: /{index} + method: PUT + parameters: + index: games +chapters: + - synopsis: Create an alias. + path: /{index}/_alias/{name} + method: PUT + parameters: + index: games + name: jeux + - synopsis: Get an alias from index. + method: GET + path: /{index}/_alias/{name} + parameters: + index: games + name: jeux + response: + status: 200 + content_type: application/json + payload: + games: + aliases: + jeux: {} + - synopsis: Delete an alias. + path: /{index}/_alias/{name} + method: DELETE + parameters: + index: games + name: jeux + - synopsis: Create an alias by Create or Update alias endpoint. + path: /{index}/_alias/{name} + method: PUT + parameters: + index: games + name: jeux + - synopsis: Create an alias with custom settings by Create or Update alias endpoint. + path: /{index}/_alias/{name} + method: PUT + parameters: + index: games + name: jeux + request_body: + payload: + index_routing: test1 + search_routing: test2 + routing: test + is_write_index: true + filter: + match_all: {} + - synopsis: Create an alias with is_hidden by Create or Update alias endpoint. + version: '>= 2.16' + path: /{index}/_alias/{name} + method: PUT + parameters: + index: games + name: jeux + request_body: + payload: + is_hidden: true diff --git a/tests/indices/aliases/aliases.yaml b/tests/indices/aliases/aliases.yaml index e68a6a4d6..a1bc2baac 100644 --- a/tests/indices/aliases/aliases.yaml +++ b/tests/indices/aliases/aliases.yaml @@ -6,10 +6,8 @@ epilogues: method: DELETE status: [200, 404] prologues: - - path: /{index} + - path: games method: PUT - parameters: - index: games chapters: - synopsis: Create an alias. path: /{index}/_aliases/{name} @@ -17,31 +15,6 @@ chapters: parameters: index: games name: jeux - - synopsis: Get an alias from index. - method: GET - path: /{index}/_alias/{name} - parameters: - index: games - name: jeux - response: - status: 200 - content_type: application/json - payload: - games: - aliases: - jeux: {} - - synopsis: Get an alias from _aliases. - method: GET - path: /_alias/{name} - parameters: - name: jeux - response: - status: 200 - content_type: application/json - payload: - games: - aliases: - jeux: {} - synopsis: Multiple alias operations. path: /_aliases method: POST @@ -61,45 +34,9 @@ chapters: status: 200 payload: acknowledged: true - - synopsis: Delete an alias from _aliases. + - synopsis: Delete an alias. path: /{index}/_aliases/{name} method: DELETE parameters: index: games name: plays1 - - synopsis: Delete an alias from _alias. - path: /{index}/_alias/{name} - method: DELETE - parameters: - index: games - name: plays2 - - synopsis: Create an alias by Create or Update alias endpoint. - path: /{index}/_alias/{name} - method: PUT - parameters: - index: games - name: jeux - - synopsis: Create an alias with custom settings by Create or Update alias endpoint. - path: /{index}/_alias/{name} - method: PUT - parameters: - index: games - name: jeux - request_body: - payload: - index_routing: test1 - search_routing: test2 - routing: test - is_write_index: true - filter: - match_all: {} - - synopsis: Create an alias with is_hidden by Create or Update alias endpoint. - version: '>= 2.16' - path: /{index}/_alias/{name} - method: PUT - parameters: - index: games - name: jeux - request_body: - payload: - is_hidden: true diff --git a/tests/indices/clone.yaml b/tests/indices/clone.yaml index cf0e5fcfc..f3132ec78 100644 --- a/tests/indices/clone.yaml +++ b/tests/indices/clone.yaml @@ -4,6 +4,12 @@ description: Test cloning an index. prologues: - path: /movies method: PUT + - path: /movies/_settings + method: PUT + request_body: + payload: + settings: + index.blocks.write: true epilogues: - path: /movies method: DELETE @@ -18,15 +24,6 @@ epilogues: method: DELETE status: [200, 404] chapters: - - synopsis: Block writes to the source index. - path: /{index}/_settings - method: PUT - parameters: - index: movies - request_body: - payload: - settings: - index.blocks.write: true - synopsis: Clone an index. path: /{index}/_clone/{target} method: POST diff --git a/tests/indices/component_template.yaml b/tests/indices/component_template.yaml index 24539cef0..c77477f0c 100644 --- a/tests/indices/component_template.yaml +++ b/tests/indices/component_template.yaml @@ -51,6 +51,8 @@ chapters: response: status: 200 - synopsis: Create an index template composed of 2 component templates. + warnings: + multiple-paths-detected: false path: /_index_template/{name} method: POST parameters: @@ -82,6 +84,8 @@ chapters: response: status: 200 - synopsis: Create an index using the composite template. + warnings: + multiple-paths-detected: false path: /{index} method: PUT parameters: @@ -89,6 +93,8 @@ chapters: response: status: 200 - synopsis: Get an index using the composite template. + warnings: + multiple-paths-detected: false path: /{index} method: GET parameters: @@ -96,6 +102,8 @@ chapters: response: status: 200 - synopsis: Delete index template. + warnings: + multiple-paths-detected: false path: /_index_template/{name} method: DELETE parameters: diff --git a/tests/indices/data_stream.yaml b/tests/indices/data_stream/data_stream.yaml similarity index 79% rename from tests/indices/data_stream.yaml rename to tests/indices/data_stream/data_stream.yaml index 4b9d282ab..d4b7952b2 100644 --- a/tests/indices/data_stream.yaml +++ b/tests/indices/data_stream/data_stream.yaml @@ -1,4 +1,4 @@ -$schema: ../../json_schemas/test_story.schema.yaml +$schema: ../../../json_schemas/test_story.schema.yaml description: Test data streams. prologues: @@ -27,6 +27,8 @@ chapters: parameters: name: logs-nginx - synopsis: Ingest data. + warnings: + multiple-paths-detected: false path: /{index}/_doc method: POST parameters: @@ -42,16 +44,6 @@ chapters: method: GET parameters: name: logs-nginx - - synopsis: Get data stream stats. - path: /_data_stream/{name}/_stats - method: GET - parameters: - name: logs-nginx - - synopsis: Manually roll over a data stream. - path: /{alias}/_rollover - method: POST - parameters: - alias: logs-nginx - synopsis: Get all data streams. path: /_data_stream method: GET diff --git a/tests/indices/data_stream/rollover.yaml b/tests/indices/data_stream/rollover.yaml new file mode 100644 index 000000000..2607b1c69 --- /dev/null +++ b/tests/indices/data_stream/rollover.yaml @@ -0,0 +1,38 @@ +$schema: ../../../json_schemas/test_story.schema.yaml + +description: Test data streams rollover. +prologues: + - path: /_index_template/logs-template-nginx + method: PUT + request_body: + payload: + index_patterns: + - logs-* + - my-data-stream + data_stream: + timestamp_field: + name: request_time + priority: 100 + - path: /_data_stream/logs-nginx + method: PUT + - path: /logs-nginx/_doc + method: POST + request_body: + payload: + message: login attempt failed + request_time: '2013-03-01T00:00:00' +epilogues: + - path: /_data_stream/logs-nginx + method: DELETE + status: [200, 404] + - path: /_index_template/logs-template + method: DELETE + status: [200, 404] +chapters: + - synopsis: Manually roll over a data stream. + warnings: + multiple-paths-detected: false + path: /{alias}/_rollover + method: POST + parameters: + alias: logs-nginx diff --git a/tests/indices/data_stream/stats.yaml b/tests/indices/data_stream/stats.yaml new file mode 100644 index 000000000..c963997be --- /dev/null +++ b/tests/indices/data_stream/stats.yaml @@ -0,0 +1,36 @@ +$schema: ../../../json_schemas/test_story.schema.yaml + +description: Test data streams. +prologues: + - path: /_index_template/logs-template-nginx + method: PUT + request_body: + payload: + index_patterns: + - logs-* + - my-data-stream + data_stream: + timestamp_field: + name: request_time + priority: 100 + - path: /_data_stream/logs-nginx + method: PUT + - path: /logs-nginx/_doc + method: POST + request_body: + payload: + message: login attempt failed + request_time: '2013-03-01T00:00:00' +epilogues: + - path: /_data_stream/logs-nginx + method: DELETE + status: [200, 404] + - path: /_index_template/logs-template + method: DELETE + status: [200, 404] +chapters: + - synopsis: Get data stream stats. + path: /_data_stream/{name}/_stats + method: GET + parameters: + name: logs-nginx diff --git a/tests/indices/index.yaml b/tests/indices/index.yaml index c9302a288..22ee0a3d7 100644 --- a/tests/indices/index.yaml +++ b/tests/indices/index.yaml @@ -71,18 +71,6 @@ chapters: index: books,games flat_settings: true - - synopsis: Close the `books` index. - path: /{index}/_close - method: POST - parameters: - index: books - - - synopsis: Open the `books` index. - path: /{index}/_open - method: POST - parameters: - index: books - - synopsis: Delete the `books` and `games` indices. path: /{index} method: DELETE diff --git a/tests/indices/shrink.yaml b/tests/indices/shrink.yaml index bfa798487..58b8fee23 100644 --- a/tests/indices/shrink.yaml +++ b/tests/indices/shrink.yaml @@ -8,6 +8,12 @@ prologues: payload: settings: index.number_of_shards: 3 + - path: /movies/_settings + method: PUT + request_body: + payload: + settings: + index.blocks.write: true epilogues: - path: /movies method: DELETE @@ -25,15 +31,6 @@ epilogues: method: DELETE status: [200, 404] chapters: - - synopsis: Block writes to the source index. - path: /{index}/_settings - method: PUT - parameters: - index: movies - request_body: - payload: - settings: - index.blocks.write: true - synopsis: Shrink an index (POST). path: /{index}/_shrink/{target} method: POST diff --git a/tests/indices/split.yaml b/tests/indices/split.yaml index 626edd8ee..ae6204060 100644 --- a/tests/indices/split.yaml +++ b/tests/indices/split.yaml @@ -8,6 +8,12 @@ prologues: payload: settings: index.number_of_shards: 3 + - path: /movies/_settings + method: PUT + request_body: + payload: + settings: + index.blocks.write: true epilogues: - path: /movies method: DELETE @@ -25,15 +31,6 @@ epilogues: method: DELETE status: [200, 404] chapters: - - synopsis: Block writes to the source index. - path: /{index}/_settings - method: PUT - parameters: - index: movies - request_body: - payload: - settings: - index.blocks.write: true - synopsis: Split an index (POST). path: /{index}/_split/{target} method: POST diff --git a/tests/ml/model_groups.yaml b/tests/ml/model_groups.yaml index 3a52127ce..46f8595d7 100644 --- a/tests/ml/model_groups.yaml +++ b/tests/ml/model_groups.yaml @@ -3,15 +3,8 @@ $schema: ../../json_schemas/test_story.schema.yaml description: | Test the creation of model groups. version: '>= 2.11' -epilogues: - - path: /_plugins/_ml/model_groups/{model_group_id} - method: DELETE - status: [200, 404] - parameters: - model_group_id: ${create_model_group.test_model_group_id} -chapters: - - synopsis: Configure cluster. - path: /_cluster/settings +prologues: + - path: /_cluster/settings method: PUT request_body: payload: @@ -19,7 +12,16 @@ chapters: plugins: ml_commons: only_run_on_ml_node: false +epilogues: + - path: /_plugins/_ml/model_groups/{model_group_id} + method: DELETE + status: [200, 404] + parameters: + model_group_id: ${create_model_group.test_model_group_id} +chapters: - synopsis: Create model group. + warnings: + multiple-paths-detected: false id: create_model_group path: /_plugins/_ml/model_groups/_register method: POST diff --git a/tests/ml/models.yaml b/tests/ml/models.yaml index ea1658e75..452f7bacf 100644 --- a/tests/ml/models.yaml +++ b/tests/ml/models.yaml @@ -29,6 +29,8 @@ chapters: - synopsis: Wait for task. path: /_plugins/_ml/tasks/{task_id} method: GET + warnings: + multiple-paths-detected: false parameters: task_id: ${create_model.task_id} response: diff --git a/tests/nodes/stats.yaml b/tests/nodes/stats.yaml index 2ac313c15..ecb6152ea 100644 --- a/tests/nodes/stats.yaml +++ b/tests/nodes/stats.yaml @@ -36,6 +36,8 @@ chapters: response: status: 200 - synopsis: Get fs metric node stats (node_id_or_metric). + warnings: + multiple-paths-detected: false path: /_nodes/{node_id_or_metric} method: GET parameters: diff --git a/tests/sql/close.yaml b/tests/sql/close.yaml index 3c23a6299..8df8a4c01 100644 --- a/tests/sql/close.yaml +++ b/tests/sql/close.yaml @@ -22,23 +22,20 @@ prologues: method: POST parameters: index: books -epilogues: - - path: /books - method: DELETE - status: [200, 404] -chapters: - - synopsis: Get SQL query. - id: query_sql + - id: query_sql path: /_plugins/_sql method: POST request_body: payload: fetch_size: 1 query: 'SELECT * FROM books' - response: - status: 200 output: cursor: payload.cursor +epilogues: + - path: /books + method: DELETE + status: [200, 404] +chapters: - synopsis: Close cursor. path: /_plugins/_sql/close method: POST diff --git a/tools/src/tester/ResultLogger.ts b/tools/src/tester/ResultLogger.ts index 0ae4dd2cf..ddfead55c 100644 --- a/tools/src/tester/ResultLogger.ts +++ b/tools/src/tester/ResultLogger.ts @@ -46,8 +46,9 @@ export class ConsoleResultLogger implements ResultLogger { console.log(`Tested ${results.evaluated_paths_count()}/${results.spec_paths_count()} paths.`) } - #log_story ({ result, full_path, display_path, message }: StoryEvaluation): void { + #log_story ({ result, full_path, display_path, message, warnings }: StoryEvaluation): void { this.#log_evaluation({ result, message: message ?? full_path }, ansi.cyan(ansi.b(display_path))) + this.#log_warnings(warnings) } #log_chapters (evaluations: ChapterEvaluation[], title: string): void { @@ -121,6 +122,11 @@ export class ConsoleResultLogger implements ResultLogger { } } + #log_warnings(warnings?: string[]): void { + if (!warnings) return + warnings.forEach((warning) => { console.log(ansi.gray(`WARNING ${(warning)}`)); }) + } + #maybe_shorten_error_message(message: string | undefined): string | undefined { if (message === undefined || message.length <= 128 || this._verbose) return message const part = message.split(',')[0] diff --git a/tools/src/tester/StoryEvaluator.ts b/tools/src/tester/StoryEvaluator.ts index e4da7b162..1e2a95d52 100644 --- a/tools/src/tester/StoryEvaluator.ts +++ b/tools/src/tester/StoryEvaluator.ts @@ -15,6 +15,7 @@ import { StoryOutputs } from './StoryOutputs' import SupplementalChapterEvaluator from './SupplementalChapterEvaluator' import { ChapterOutput } from './ChapterOutput' import * as semver from 'semver' +import _ from 'lodash' export default class StoryEvaluator { private readonly _chapter_evaluator: ChapterEvaluator @@ -51,7 +52,27 @@ export default class StoryEvaluator { chapters, prologues, epilogues, - result: overall_result(prologues.concat(chapters).concat(epilogues).concat(prologues).map(e => e.overall)) + result: overall_result(prologues.concat(chapters).concat(epilogues).concat(prologues).map(e => e.overall)), + warnings: this.#chapter_warnings(story.chapters) + } + } + + #chapter_warnings(chapters: Chapter[]): string[] | undefined { + const result = _.compact([ + this.#warning_if_mismatched_chapter_paths(chapters) + ]) + return result.length > 0 ? result : undefined + } + + #warning_if_mismatched_chapter_paths(chapters: Chapter[]): string | undefined { + const paths = _.compact(_.map(chapters, (chapter) => { + const multiple_paths_detected = chapter.warnings?.['multiple-paths-detected'] ?? true + if (multiple_paths_detected) return chapter.path + })) + const normalized_paths = _.map(paths, (path) => path.replaceAll(/\/\{[^}]+}/g, '').replaceAll('//', '/')) + const paths_counts: Record = Object.assign((_.values(_.groupBy(normalized_paths)).map(p => { return { [p[0]] : p.length } }))) + if (paths_counts.length > 1) { + return `Multiple paths detected, please group similar tests together and move paths not being tested to prologues or epilogues.\n ${_.join(_.uniq(paths), "\n ")}\n` } } diff --git a/tools/src/tester/types/eval.types.ts b/tools/src/tester/types/eval.types.ts index 7c7f751bf..a89352eb0 100644 --- a/tools/src/tester/types/eval.types.ts +++ b/tools/src/tester/types/eval.types.ts @@ -26,6 +26,7 @@ export interface StoryEvaluation { chapters?: ChapterEvaluation[] epilogues?: ChapterEvaluation[] prologues?: ChapterEvaluation[] + warnings?: string[] } export interface StoryEvaluations { diff --git a/tools/src/tester/types/story.types.ts b/tools/src/tester/types/story.types.ts index 5ec1c5475..3b7fbe18b 100644 --- a/tools/src/tester/types/story.types.ts +++ b/tools/src/tester/types/story.types.ts @@ -72,6 +72,7 @@ export type Chapter = ChapterRequest & { */ synopsis: string; response?: ExpectedResponse; + warnings?: Warnings; }; /** * This interface was referenced by `Story`'s JSON-Schema @@ -143,6 +144,13 @@ export interface ExpectedResponse { content_type?: string; payload?: Payload; } +/** + * This interface was referenced by `Story`'s JSON-Schema + * via the `definition` "Warnings". + */ +export interface Warnings { + 'multiple-paths-detected'?: boolean; +} /** * This interface was referenced by `Story`'s JSON-Schema * via the `definition` "ActualResponse". diff --git a/tools/tests/tester/ResultLogger.test.ts b/tools/tests/tester/ResultLogger.test.ts index 6f454dcb3..f21e45af1 100644 --- a/tools/tests/tester/ResultLogger.test.ts +++ b/tools/tests/tester/ResultLogger.test.ts @@ -132,5 +132,30 @@ describe('ConsoleResultLogger', () => { [`${ansi.green('PASSED ')} ${ansi.cyan(ansi.b('path'))} ${ansi.gray('(message)')}`] ]) }) + + describe('with warnings', () => { + const logger = new ConsoleResultLogger(tab_width, true) + + test('log', () => { + logger.log({ + result: Result.PASSED, + display_path: 'path', + full_path: 'full_path', + description: 'description', + message: 'message', + warnings: ['warn1', 'warn2'], + epilogues: [], + prologues: [] + }) + + expect(log.mock.calls).toEqual([ + [], + [`${ansi.green('PASSED ')} ${ansi.cyan(ansi.b('path'))} ${ansi.gray('(message)')}`], + [ansi.gray("WARNING warn1")], + [ansi.gray("WARNING warn2")], + [] + ]) + }) + }) }) }) diff --git a/tools/tests/tester/fixtures/evals/error/chapter_error.yaml b/tools/tests/tester/fixtures/evals/error/chapter_error.yaml index 9ea8e1c75..775bffb2e 100644 --- a/tools/tests/tester/fixtures/evals/error/chapter_error.yaml +++ b/tools/tests/tester/fixtures/evals/error/chapter_error.yaml @@ -4,6 +4,12 @@ full_path: tools/tests/tester/fixtures/stories/error/chapter_error.yaml result: ERROR description: This story should failed due to missing info in the spec. +warnings: + - | + Multiple paths detected, please group similar tests together and move paths not being tested to prologues or epilogues. + /{index}/settings + /{index} + /_cat/indices prologues: - title: PUT /books overall: diff --git a/tools/tests/tester/fixtures/evals/error/output_error.yaml b/tools/tests/tester/fixtures/evals/error/output_error.yaml index 32afe57cf..ed4e57ac3 100644 --- a/tools/tests/tester/fixtures/evals/error/output_error.yaml +++ b/tools/tests/tester/fixtures/evals/error/output_error.yaml @@ -5,6 +5,7 @@ result: ERROR description: This story has an error in the output. prologues: [] epilogues: [] + chapters: - title: This chapter expects a `cursor` in the output. overall: diff --git a/tools/tests/tester/fixtures/evals/error/prologue_error.yaml b/tools/tests/tester/fixtures/evals/error/prologue_error.yaml index 299fa9fee..5ff63bb87 100644 --- a/tools/tests/tester/fixtures/evals/error/prologue_error.yaml +++ b/tools/tests/tester/fixtures/evals/error/prologue_error.yaml @@ -3,7 +3,11 @@ full_path: tools/tests/tester/fixtures/stories/error/prologue_error.yaml result: ERROR description: This story should failed due to missing info in the spec. - +warnings: + - | + Multiple paths detected, please group similar tests together and move paths not being tested to prologues or epilogues. + /_cat/health + /_cat/indices prologues: - title: PUT /books overall: diff --git a/tools/tests/tester/fixtures/evals/failed/invalid_data.yaml b/tools/tests/tester/fixtures/evals/failed/invalid_data.yaml index 10782489d..91aeca18b 100644 --- a/tools/tests/tester/fixtures/evals/failed/invalid_data.yaml +++ b/tools/tests/tester/fixtures/evals/failed/invalid_data.yaml @@ -3,9 +3,7 @@ full_path: tools/tests/tester/fixtures/stories/failed/invalid_data.yaml result: ERROR description: This story should failed due invalid data. - prologues: [] - chapters: - title: This chapter should fail because the parameter is invalid. overall: diff --git a/tools/tests/tester/fixtures/evals/failed/not_found.yaml b/tools/tests/tester/fixtures/evals/failed/not_found.yaml index 378cb0686..ec760c0b1 100644 --- a/tools/tests/tester/fixtures/evals/failed/not_found.yaml +++ b/tools/tests/tester/fixtures/evals/failed/not_found.yaml @@ -3,9 +3,12 @@ full_path: tools/tests/tester/fixtures/stories/failed/not_found.yaml result: FAILED description: This story should failed due to missing info in the spec. - prologues: [] - +warnings: + - | + Multiple paths detected, please group similar tests together and move paths not being tested to prologues or epilogues. + /_cat/does_not_exist + /{index} chapters: - title: This chapter should fail because the operation is not defined in the spec. overall: diff --git a/tools/tests/tester/fixtures/evals/passed.yaml b/tools/tests/tester/fixtures/evals/passed.yaml index 865df7b0b..f0118f111 100644 --- a/tools/tests/tester/fixtures/evals/passed.yaml +++ b/tools/tests/tester/fixtures/evals/passed.yaml @@ -3,9 +3,7 @@ full_path: tools/tests/tester/fixtures/stories/passed.yaml result: PASSED description: This story should pass. - prologues: [] - chapters: - title: This PUT /{index} chapter should pass. overall: diff --git a/tools/tests/tester/fixtures/evals/skipped/semver.yaml b/tools/tests/tester/fixtures/evals/skipped/semver.yaml index c6ae7a4de..b39460dcb 100644 --- a/tools/tests/tester/fixtures/evals/skipped/semver.yaml +++ b/tools/tests/tester/fixtures/evals/skipped/semver.yaml @@ -4,3 +4,4 @@ full_path: tools/tests/tester/fixtures/stories/skipped/semver.yaml result: SKIPPED description: This story should be skipped because of version. message: Skipped because version 2.15.0 does not satisfy >= 2.999.0. + diff --git a/tools/tests/tester/fixtures/stories/failed/invalid_data.yaml b/tools/tests/tester/fixtures/stories/failed/invalid_data.yaml index 404c81f28..ba84d4200 100644 --- a/tools/tests/tester/fixtures/stories/failed/invalid_data.yaml +++ b/tools/tests/tester/fixtures/stories/failed/invalid_data.yaml @@ -24,6 +24,8 @@ chapters: aliases: {} - synopsis: This chapter should fail because the response content type does not match. path: /_cat/indices/{index} + warnings: + multiple-paths-detected: false method: GET parameters: index: books diff --git a/tools/tests/tester/fixtures/stories/passed.yaml b/tools/tests/tester/fixtures/stories/passed.yaml index 65b6a3826..b1d458f25 100644 --- a/tools/tests/tester/fixtures/stories/passed.yaml +++ b/tools/tests/tester/fixtures/stories/passed.yaml @@ -9,11 +9,15 @@ chapters: - synopsis: This PUT /{index} chapter should pass. path: /{index} method: PUT + warnings: + multiple-paths-detected: false parameters: index: books - synopsis: This GET /_cat chapter returns text/plain and should pass. path: /_cat method: GET + warnings: + multiple-paths-detected: false response: status: 200 content_type: text/plain diff --git a/tools/tests/tester/integ/StoryEvaluator.test.ts b/tools/tests/tester/integ/StoryEvaluator.test.ts index ab70d3d46..7abc9dbfd 100644 --- a/tools/tests/tester/integ/StoryEvaluator.test.ts +++ b/tools/tests/tester/integ/StoryEvaluator.test.ts @@ -40,9 +40,20 @@ test('error/prologue_error', async () => { expect(actual).toEqual(expected) }) +test('error/output_error', async () => { + const actual = await load_actual_evaluation(story_evaluator, 'error/output_error') + const expected = load_expected_evaluation('error/output_error') + expect(actual).toEqual(expected) +}) + test('error/chapter_error', async () => { const actual = await load_actual_evaluation(story_evaluator, 'error/chapter_error') const expected = load_expected_evaluation('error/chapter_error') expect(actual).toEqual(expected) }) +test('skipped/semver', async () => { + const actual = await load_actual_evaluation(story_evaluator, 'skipped/semver') + const expected = load_expected_evaluation('skipped/semver') + expect(actual).toEqual(expected) +})