From 6081d1ae0600bc82c731be143e543ae39cdfd3d3 Mon Sep 17 00:00:00 2001 From: Theo Nam Truong Date: Wed, 26 Jun 2024 14:49:09 -0600 Subject: [PATCH] Added StoryValidator (#362) Validating stories before running them. Signed-off-by: Theo Truong --- CHANGELOG.md | 1 + eslint.config.mjs | 2 + json_schemas/test_story.schema.yaml | 11 ++-- package.json | 4 +- tests/_core/bulk.yaml | 1 - tests/_core/search.yaml | 4 +- tests/cat/index.yaml | 1 - tools/src/linter/components/NamespaceFile.ts | 2 +- .../components/SupersededOperationsFile.ts | 2 +- tools/src/tester/ChapterEvaluator.ts | 2 +- tools/src/tester/ChapterReader.ts | 2 +- tools/src/tester/ResultLogger.ts | 8 ++- tools/src/tester/SchemaValidator.ts | 2 +- tools/src/tester/StoryEvaluator.ts | 29 ++++------ tools/src/tester/StoryValidator.ts | 55 +++++++++++++++++++ tools/src/tester/TestRunner.ts | 9 ++- tools/src/tester/test.ts | 4 +- tools/src/tester/types/eval.types.ts | 9 ++- tools/src/tester/types/story.types.ts | 5 +- tools/tests/merger/OpenApiMerger.test.ts | 4 +- tools/tests/tester/ResultLogger.test.ts | 8 +-- tools/tests/tester/StoryValidator.test.ts | 39 +++++++++++++ .../tester/fixtures/empty_story/empty.yaml | 3 + .../fixtures/empty_with_all_the_parts.yaml | 2 +- .../fixtures/empty_with_description.yaml | 5 -- .../fixtures/fail_non_existent_variable.yaml | 2 +- .../tests/tester/fixtures/invalid_story.yaml | 14 +++++ tools/tests/tester/fixtures/valid_story.yaml | 13 +++++ .../tests/tester/fixtures/wrong_$schema.yaml | 1 + tools/tests/tester/helpers.ts | 6 +- tools/tests/tester/test.test.ts | 17 ++++-- 31 files changed, 198 insertions(+), 69 deletions(-) create mode 100644 tools/src/tester/StoryValidator.ts create mode 100644 tools/tests/tester/StoryValidator.test.ts delete mode 100644 tools/tests/tester/fixtures/empty_with_description.yaml create mode 100644 tools/tests/tester/fixtures/invalid_story.yaml create mode 100644 tools/tests/tester/fixtures/valid_story.yaml create mode 100644 tools/tests/tester/fixtures/wrong_$schema.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d386c79..adb84c2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Added `cancel_after_time_interval` and `phase_took` in `_search` ([#353](https://github.com/opensearch-project/opensearch-api-specification/pull/353)) - Added support for testing `application/x-ndjson` payloads ([#355](https://github.com/opensearch-project/opensearch-api-specification/pull/355)) - Added SPECIFICATION_TESTING.md [#359](https://github.com/opensearch-project/opensearch-api-specification/pull/359) +- Added StoryValidator to validate stories before running them ([#354](https://github.com/opensearch-project/opensearch-api-specification/issues/354)) ### Changed diff --git a/eslint.config.mjs b/eslint.config.mjs index e97f226c..758ce7a9 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -35,6 +35,8 @@ export default [ ...pluginComments.configs.recommended.rules, ...pluginTs.configs["recommended-type-checked"].rules, '@stylistic/object-curly-spacing': ["error", "always"], + '@stylistic/keyword-spacing': ["error", { "before": true, "after": true }], + '@stylistic/space-unary-ops': ["error", { "words": true, "nonwords": false }], '@typescript-eslint/no-unsafe-member-access': 'off', '@typescript-eslint/no-explicit-any': 'off', '@typescript-eslint/no-unsafe-assignment': 'off', diff --git a/json_schemas/test_story.schema.yaml b/json_schemas/test_story.schema.yaml index d0716669..a4cddf37 100644 --- a/json_schemas/test_story.schema.yaml +++ b/json_schemas/test_story.schema.yaml @@ -1,4 +1,4 @@ -$schema: http://json-schema.org/draft-07/schema# +$schema: https://json-schema.org/draft/2019-09/schema type: object properties: @@ -18,13 +18,12 @@ properties: type: array items: $ref: '#/definitions/Chapter' - minItems: 1 -required: [description, chapters] +required: [$schema, description, chapters] additionalProperties: false definitions: Chapter: - type: object + unevaluatedProperties: false allOf: - $ref: '#/definitions/ChapterRequest' - properties: @@ -34,7 +33,6 @@ definitions: response: $ref: '#/definitions/ExpectedResponse' required: [synopsis] - additionalProperties: false ReadChapter: allOf: @@ -47,6 +45,7 @@ definitions: additionalProperties: false SupplementalChapter: + unevaluatedProperties: false allOf: - $ref: '#/definitions/ChapterRequest' - type: object @@ -57,7 +56,6 @@ definitions: default: [200, 201] items: type: integer - additionalProperties: false ChapterRequest: type: object @@ -79,7 +77,6 @@ definitions: output: $ref: '#/definitions/Output' required: [path, method] - additionalProperties: false Output: description: | diff --git a/package.json b/package.json index f614e34e..c67e8379 100644 --- a/package.json +++ b/package.json @@ -12,10 +12,10 @@ "lint--fix": "eslint . --fix --report-unused-disable-directives", "merge": "ts-node tools/src/merger/merge.ts", "test": "npm run test:unit && npm run test:integ", - "jest": "jest", "test:unit": "jest --testMatch='**/*.test.ts' --testPathIgnorePatterns=/integ/", "test:integ": "jest --testMatch='**/integ/*.test.ts' --runInBand", - "test:spec": "ts-node tools/src/tester/test.ts" + "test:spec": "ts-node tools/src/tester/test.ts", + "test:spec--insecure": "ts-node tools/src/tester/test.ts --opensearch-insecure" }, "dependencies": { "@apidevtools/swagger-parser": "^10.1.0", diff --git a/tests/_core/bulk.yaml b/tests/_core/bulk.yaml index 9bd3091a..04e03e24 100644 --- a/tests/_core/bulk.yaml +++ b/tests/_core/bulk.yaml @@ -1,6 +1,5 @@ $schema: ../../json_schemas/test_story.schema.yaml -skip: false description: Test bulk endpoint. epilogues: - path: /books,movies diff --git a/tests/_core/search.yaml b/tests/_core/search.yaml index f92613c7..6c9ef9f4 100644 --- a/tests/_core/search.yaml +++ b/tests/_core/search.yaml @@ -1,6 +1,5 @@ $schema: ../../json_schemas/test_story.schema.yaml -skip: false description: Test search endpoint. prologues: - path: /movies/_doc @@ -12,8 +11,7 @@ prologues: director: Bennett Miller title: Moneyball year: 2011 - response: - status: 201 + status: [201] epilogues: - path: /movies method: DELETE diff --git a/tests/cat/index.yaml b/tests/cat/index.yaml index cbe2e228..7127fe57 100644 --- a/tests/cat/index.yaml +++ b/tests/cat/index.yaml @@ -1,6 +1,5 @@ $schema: ../../json_schemas/test_story.schema.yaml -skip: false description: Test cat endpoints. chapters: - synopsis: Cat with a json response. diff --git a/tools/src/linter/components/NamespaceFile.ts b/tools/src/linter/components/NamespaceFile.ts index fb6ad52f..fdfb2d0e 100644 --- a/tools/src/linter/components/NamespaceFile.ts +++ b/tools/src/linter/components/NamespaceFile.ts @@ -116,7 +116,7 @@ export default class NamespaceFile extends FileValidator { const current_keys = _.keys(p) sort_by_keys(p as Record, HTTP_METHODS) const sorted_keys = _.keys(p) - if(!_.isEqual(current_keys, sorted_keys)) { + if (!_.isEqual(current_keys, sorted_keys)) { return this.error( `Operations must be sorted. Expected ${_.join(sorted_keys, ', ')}.`, path) diff --git a/tools/src/linter/components/SupersededOperationsFile.ts b/tools/src/linter/components/SupersededOperationsFile.ts index 65e3ec82..faa3748b 100644 --- a/tools/src/linter/components/SupersededOperationsFile.ts +++ b/tools/src/linter/components/SupersededOperationsFile.ts @@ -36,7 +36,7 @@ export default class SupersededOperationsFile extends FileValidator { return _.entries(this.superseded_ops()).map(([path, p]) => { const current_keys = p.operations const sorted_keys = sort_array_by_keys(p.operations as string[], HTTP_METHODS) - if(!_.isEqual(current_keys, sorted_keys)) { + if (!_.isEqual(current_keys, sorted_keys)) { return this.error( `Operations must be sorted. Expected ${_.join(sorted_keys, ', ')}.`, path) diff --git a/tools/src/tester/ChapterEvaluator.ts b/tools/src/tester/ChapterEvaluator.ts index 980ca751..c9f60f50 100644 --- a/tools/src/tester/ChapterEvaluator.ts +++ b/tools/src/tester/ChapterEvaluator.ts @@ -87,7 +87,7 @@ export default class ChapterEvaluator { 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) { + switch (value.type) { case Operation.UPDATE: return `expected ${value.path.replace('$.', '')}='${value.oldValue}', got '${value.value}'` case Operation.REMOVE: diff --git a/tools/src/tester/ChapterReader.ts b/tools/src/tester/ChapterReader.ts index e34edb17..9ac029ee 100644 --- a/tools/src/tester/ChapterReader.ts +++ b/tools/src/tester/ChapterReader.ts @@ -61,7 +61,7 @@ export default class ChapterReader { #serialize_payload(payload: any, content_type: string): any { if (payload === undefined) return undefined - switch(content_type) { + switch (content_type) { case 'application/x-ndjson': return to_ndjson(payload as any[]) default: return payload } diff --git a/tools/src/tester/ResultLogger.ts b/tools/src/tester/ResultLogger.ts index 2a1aef7f..4d9eb723 100644 --- a/tools/src/tester/ResultLogger.ts +++ b/tools/src/tester/ResultLogger.ts @@ -29,15 +29,17 @@ export class ConsoleResultLogger implements ResultLogger { } log (evaluation: StoryEvaluation): void { + const with_padding = [Result.FAILED, Result.ERROR].includes(evaluation.result) || this._verbose + if (with_padding) console.log() this.#log_story(evaluation) this.#log_chapters(evaluation.prologues ?? [], 'PROLOGUES') this.#log_chapters(evaluation.chapters ?? [], 'CHAPTERS') this.#log_chapters(evaluation.epilogues ?? [], 'EPILOGUES') - console.log('\n') + if (with_padding) console.log() } - #log_story ({ result, full_path, description, display_path, message }: StoryEvaluation): void { - this.#log_evaluation({ result, message: message ?? full_path }, ansi.cyan(ansi.b(description ?? display_path))) + #log_story ({ result, full_path, display_path, message }: StoryEvaluation): void { + this.#log_evaluation({ result, message: message ?? full_path }, ansi.cyan(ansi.b(display_path))) } #log_chapters (evaluations: ChapterEvaluation[], title: string): void { diff --git a/tools/src/tester/SchemaValidator.ts b/tools/src/tester/SchemaValidator.ts index 25fe3d5d..c8f13e09 100644 --- a/tools/src/tester/SchemaValidator.ts +++ b/tools/src/tester/SchemaValidator.ts @@ -32,7 +32,7 @@ export default class SchemaValidator { validate (schema: OpenAPIV3.SchemaObject, data: any): Evaluation { const validate = this.ajv.compile(schema) const valid = validate(data) - if (! valid) { + if (!valid) { this.logger.info(`# ${to_json(schema)}`) this.logger.info(`* ${to_json(data)}`) this.logger.info(`& ${to_json(validate.errors)}`) diff --git a/tools/src/tester/StoryEvaluator.ts b/tools/src/tester/StoryEvaluator.ts index 3e602a1c..ecc6d3b6 100644 --- a/tools/src/tester/StoryEvaluator.ts +++ b/tools/src/tester/StoryEvaluator.ts @@ -8,19 +8,13 @@ */ import { ChapterRequest, Parameter, type Chapter, type Story, type SupplementalChapter } from './types/story.types' -import { type ChapterEvaluation, Result, type StoryEvaluation, OutputReference } from './types/eval.types' +import { type StoryFile, type ChapterEvaluation, Result, type StoryEvaluation, OutputReference } from './types/eval.types' import type ChapterEvaluator from './ChapterEvaluator' import { overall_result } from './helpers' import { StoryOutputs } from './StoryOutputs' import SupplementalChapterEvaluator from './SupplementalChapterEvaluator' import { ChapterOutput } from './ChapterOutput' -export interface StoryFile { - display_path: string - full_path: string - story: Story -} - export default class StoryEvaluator { private readonly _chapter_evaluator: ChapterEvaluator private readonly _supplemental_chapter_evaluator: SupplementalChapterEvaluator @@ -87,16 +81,17 @@ export default class StoryEvaluator { return { evaluations, has_errors } } + // TODO: Refactor and move this logic into StoryValidator static check_story_variables(story: Story, display_path: string, full_path: string): StoryEvaluation | undefined { const story_outputs = new StoryOutputs() const prologues = (story.prologues ?? []).map((prologue) => { - return StoryEvaluator.#check_episode_variables(prologue, story_outputs) + return StoryEvaluator.#check_chapter_variables(prologue, story_outputs) }) const chapters = (story.chapters ?? []).map((chapter) => { - return StoryEvaluator.#check_episode_variables(chapter, story_outputs) + return StoryEvaluator.#check_chapter_variables(chapter, story_outputs) }) const epilogues = (story.epilogues ?? []).map((epilogue) => { - return StoryEvaluator.#check_episode_variables(epilogue, story_outputs) + return StoryEvaluator.#check_chapter_variables(epilogue, story_outputs) }) const evals = prologues.concat(chapters).concat(epilogues) if (overall_result(evals.map(e => e.overall)) === Result.FAILED) { @@ -113,17 +108,17 @@ export default class StoryEvaluator { } } - static #check_episode_variables(episode: ChapterRequest, story_outputs: StoryOutputs): ChapterEvaluation { - const title = `${episode.method} ${episode.path}` - const error = StoryEvaluator.#check_used_variables(episode, story_outputs) + static #check_chapter_variables(chapter: ChapterRequest, story_outputs: StoryOutputs): ChapterEvaluation { + const title = `${chapter.method} ${chapter.path}` + const error = StoryEvaluator.#check_used_variables(chapter, story_outputs) if (error !== undefined) { return error } - if (episode.id === undefined && episode.output !== undefined) { - return this.#failed_evaluation(title, 'An episode must have an id to store its output') + if (chapter.id === undefined && chapter.output !== undefined) { + return this.#failed_evaluation(title, 'A chapter must have an id to store its output') } - if (episode.id !== undefined && episode.output !== undefined) { - story_outputs.set_chapter_output(episode.id, new ChapterOutput(episode.output)) + if (chapter.id !== undefined && chapter.output !== undefined) { + story_outputs.set_chapter_output(chapter.id, new ChapterOutput(chapter.output)) } return { title, overall: { result: Result.PASSED } } } diff --git a/tools/src/tester/StoryValidator.ts b/tools/src/tester/StoryValidator.ts new file mode 100644 index 00000000..6b15228d --- /dev/null +++ b/tools/src/tester/StoryValidator.ts @@ -0,0 +1,55 @@ +/* +* Copyright OpenSearch Contributors +* SPDX-License-Identifier: Apache-2.0 +* +* The OpenSearch Contributors require contributions made to +* this file be licensed under the Apache-2.0 license or a +* compatible open source license. +*/ + +import { Result, StoryEvaluation, StoryFile } from "./types/eval.types"; +import * as path from "path"; +import { Ajv2019, ValidateFunction } from 'ajv/dist/2019' +import addFormats from 'ajv-formats' +import { read_yaml } from "../helpers"; + +export default class StoryValidator { + private static readonly SCHEMA_FILE = path.resolve("./json_schemas/test_story.schema.yaml") + private readonly ajv: Ajv2019 + private readonly validate_schema: ValidateFunction + + constructor() { + this.ajv = new Ajv2019({ allErrors: true, strict: false }) + addFormats(this.ajv) + const schema = read_yaml(StoryValidator.SCHEMA_FILE) + this.validate_schema = this.ajv.compile(schema) + } + + validate(story_file: StoryFile): StoryEvaluation | undefined { + const schema_file_error = this.#validate_schema_path(story_file) + if (schema_file_error != null) return schema_file_error + const schema_error = this.#validate_schema(story_file) + if (schema_error != null) return schema_error + } + + #validate_schema_path(story_file: StoryFile): StoryEvaluation | undefined { + const actual_path = story_file.story.$schema + const expected_path = path.relative(path.dirname(story_file.full_path), StoryValidator.SCHEMA_FILE) + if (actual_path !== expected_path) return this.#invalid(story_file, `Expected $schema to be ${expected_path}`) + } + + #validate_schema(story_file: StoryFile): StoryEvaluation | undefined { + const valid = this.validate_schema(story_file.story) + if (!valid) return this.#invalid(story_file, this.ajv.errorsText(this.validate_schema.errors)) + } + + #invalid({ story, display_path, full_path }: StoryFile, message: string): StoryEvaluation { + return { + display_path, + full_path, + description: story.description, + result: Result.ERROR, + message: `Invalid Story: ${message}`, + } + } +} \ No newline at end of file diff --git a/tools/src/tester/TestRunner.ts b/tools/src/tester/TestRunner.ts index 779d314d..87ac31c8 100644 --- a/tools/src/tester/TestRunner.ts +++ b/tools/src/tester/TestRunner.ts @@ -8,19 +8,22 @@ */ import type StoryEvaluator from './StoryEvaluator' -import { type StoryFile } from './StoryEvaluator' +import { type StoryFile } from './types/eval.types' import fs from 'fs' import { type Story } from './types/story.types' import { read_yaml } from '../helpers' import { Result, type StoryEvaluation } from './types/eval.types' import { type ResultLogger } from './ResultLogger' import { basename, resolve } from 'path' +import type StoryValidator from "./StoryValidator"; export default class TestRunner { + private readonly _story_validator: StoryValidator private readonly _story_evaluator: StoryEvaluator private readonly _result_logger: ResultLogger - constructor (story_evaluator: StoryEvaluator, result_logger: ResultLogger) { + constructor (story_validator: StoryValidator, story_evaluator: StoryEvaluator, result_logger: ResultLogger) { + this._story_validator = story_validator this._story_evaluator = story_evaluator this._result_logger = result_logger } @@ -30,7 +33,7 @@ export default class TestRunner { const story_files = this.#sort_story_files(this.#collect_story_files(resolve(story_path), '', '')) const evaluations: StoryEvaluation[] = [] for (const story_file of story_files) { - const evaluation = await this._story_evaluator.evaluate(story_file, dry_run) + const evaluation = this._story_validator.validate(story_file) ?? await this._story_evaluator.evaluate(story_file, dry_run) evaluations.push(evaluation) this._result_logger.log(evaluation) if ([Result.ERROR, Result.FAILED].includes(evaluation.result)) failed = true diff --git a/tools/src/tester/test.ts b/tools/src/tester/test.ts index 15db9821..6c84c2b3 100644 --- a/tools/src/tester/test.ts +++ b/tools/src/tester/test.ts @@ -27,6 +27,7 @@ import { ConsoleResultLogger } from './ResultLogger' import * as process from 'node:process' import SupplementalChapterEvaluator from './SupplementalChapterEvaluator' import MergedOpenApiSpec from './MergedOpenApiSpec' +import StoryValidator from "./StoryValidator"; const command = new Command() .description('Run test stories against the OpenSearch spec.') @@ -54,9 +55,10 @@ const http_client = new OpenSearchHttpClient(get_opensearch_opts_from_cli(opts)) const chapter_reader = new ChapterReader(http_client, logger) const chapter_evaluator = new ChapterEvaluator(new OperationLocator(spec), chapter_reader, new SchemaValidator(spec, logger)) const supplemental_chapter_evaluator = new SupplementalChapterEvaluator(chapter_reader) +const story_validator = new StoryValidator() const story_evaluator = new StoryEvaluator(chapter_evaluator, supplemental_chapter_evaluator) const result_logger = new ConsoleResultLogger(opts.tabWidth, opts.verbose) -const runner = new TestRunner(story_evaluator, result_logger) +const runner = new TestRunner(story_validator, story_evaluator, result_logger) runner.run(opts.testsPath, opts.dryRun) .then( diff --git a/tools/src/tester/types/eval.types.ts b/tools/src/tester/types/eval.types.ts index d42e3991..f8fe386f 100644 --- a/tools/src/tester/types/eval.types.ts +++ b/tools/src/tester/types/eval.types.ts @@ -9,8 +9,13 @@ import { type ChapterOutput } from '../ChapterOutput' import { StoryOutputs } from '../StoryOutputs' +import type { Story } from "./story.types"; -export type LibraryEvaluation = StoryEvaluation[] +export interface StoryFile { + display_path: string + full_path: string + story: Story +} export interface StoryEvaluation { result: Result @@ -18,7 +23,7 @@ export interface StoryEvaluation { full_path: string description: string message?: string - chapters: ChapterEvaluation[] + chapters?: ChapterEvaluation[] epilogues?: ChapterEvaluation[] prologues?: ChapterEvaluation[] } diff --git a/tools/src/tester/types/story.types.ts b/tools/src/tester/types/story.types.ts index 257eb3ca..54641cc7 100644 --- a/tools/src/tester/types/story.types.ts +++ b/tools/src/tester/types/story.types.ts @@ -57,13 +57,10 @@ export type ReadChapter = Chapter & { }; export interface Story { - $schema?: string; + $schema: string; description: string; prologues?: SupplementalChapter[]; epilogues?: SupplementalChapter[]; - /** - * @minItems 1 - */ chapters: Chapter[]; } /** diff --git a/tools/tests/merger/OpenApiMerger.test.ts b/tools/tests/merger/OpenApiMerger.test.ts index 4e5acc20..72eeef04 100644 --- a/tools/tests/merger/OpenApiMerger.test.ts +++ b/tools/tests/merger/OpenApiMerger.test.ts @@ -9,10 +9,10 @@ import OpenApiMerger from 'merger/OpenApiMerger' import fs from 'fs' -import { Logger } from 'Logger' +import { Logger, LogLevel } from 'Logger' test('merge()', () => { - const merger = new OpenApiMerger('./tools/tests/merger/fixtures/spec/', new Logger()) + const merger = new OpenApiMerger('./tools/tests/merger/fixtures/spec/', new Logger(LogLevel.error)) merger.merge('./tools/tests/merger/opensearch-openapi.yaml') expect(fs.readFileSync('./tools/tests/merger/fixtures/expected.yaml', 'utf8')) .toEqual(fs.readFileSync('./tools/tests/merger/opensearch-openapi.yaml', 'utf8')) diff --git a/tools/tests/tester/ResultLogger.test.ts b/tools/tests/tester/ResultLogger.test.ts index 070f67a1..5cc8975d 100644 --- a/tools/tests/tester/ResultLogger.test.ts +++ b/tools/tests/tester/ResultLogger.test.ts @@ -45,10 +45,11 @@ describe('ConsoleResultLogger', () => { }) expect(log.mock.calls).toEqual([ - [`${ansi.green('PASSED ')} ${ansi.cyan(ansi.b('description'))} ${ansi.gray('(message)')}`], + [], + [`${ansi.green('PASSED ')} ${ansi.cyan(ansi.b('path'))} ${ansi.gray('(message)')}`], [` ${ansi.green('PASSED ')} CHAPTERS `], [` ${ansi.green('PASSED ')} ${ansi.i('title')} `], - ["\n"] + [] ]) }) }) @@ -74,8 +75,7 @@ describe('ConsoleResultLogger', () => { }) expect(log.mock.calls).toEqual([ - [`${ansi.green('PASSED ')} ${ansi.cyan(ansi.b('description'))} ${ansi.gray('(message)')}`], - ["\n"] + [`${ansi.green('PASSED ')} ${ansi.cyan(ansi.b('path'))} ${ansi.gray('(message)')}`] ]) }) }) diff --git a/tools/tests/tester/StoryValidator.test.ts b/tools/tests/tester/StoryValidator.test.ts new file mode 100644 index 00000000..c3a42d37 --- /dev/null +++ b/tools/tests/tester/StoryValidator.test.ts @@ -0,0 +1,39 @@ +/* +* Copyright OpenSearch Contributors +* SPDX-License-Identifier: Apache-2.0 +* +* The OpenSearch Contributors require contributions made to +* this file be licensed under the Apache-2.0 license or a +* compatible open source license. +*/ + +import StoryValidator from "../../src/tester/StoryValidator"; +import { StoryEvaluation } from "../../src/tester/types/eval.types"; +import { read_yaml } from "../../src/helpers"; +import { Story } from "../../src/tester/types/story.types"; + +const validator = new StoryValidator() + +function validate(path: string): StoryEvaluation | undefined { + const story: Story = read_yaml(path) + return validator.validate({ story, display_path: path, full_path: path }) +} + +describe('StoryValidator', () => { + test('wrong $schema', () => { + const evaluation = validate('tools/tests/tester/fixtures/wrong_$schema.yaml') + expect(evaluation?.result).toBe('ERROR') + expect(evaluation?.message).toBe('Invalid Story: Expected $schema to be ../../../../json_schemas/test_story.schema.yaml') + }) + + test('invalid story', () => { + const evaluation = validate('tools/tests/tester/fixtures/invalid_story.yaml') + expect(evaluation?.result).toBe('ERROR') + expect(evaluation?.message).toBe("Invalid Story: data/epilogues/0 must NOT have unevaluated properties, data/chapters/0 must have required property 'method', data/chapters/1/method must be equal to one of the allowed values") + }) + + test('valid story', () => { + const evaluation = validate('tools/tests/tester/fixtures/valid_story.yaml') + expect(evaluation).toBeUndefined() + }) +}) \ No newline at end of file diff --git a/tools/tests/tester/fixtures/empty_story/empty.yaml b/tools/tests/tester/fixtures/empty_story/empty.yaml index efe320f5..920a2310 100644 --- a/tools/tests/tester/fixtures/empty_story/empty.yaml +++ b/tools/tests/tester/fixtures/empty_story/empty.yaml @@ -1 +1,4 @@ +$schema: ../../../../../json_schemas/test_story.schema.yaml + +description: Empty story. chapters: [] \ No newline at end of file diff --git a/tools/tests/tester/fixtures/empty_with_all_the_parts.yaml b/tools/tests/tester/fixtures/empty_with_all_the_parts.yaml index f1eeaeb7..ca8e1e31 100644 --- a/tools/tests/tester/fixtures/empty_with_all_the_parts.yaml +++ b/tools/tests/tester/fixtures/empty_with_all_the_parts.yaml @@ -1,4 +1,4 @@ -$schema: ../json_schemas/test_story.schema.yaml +$schema: ../../../../json_schemas/test_story.schema.yaml description: A story with all its parts. diff --git a/tools/tests/tester/fixtures/empty_with_description.yaml b/tools/tests/tester/fixtures/empty_with_description.yaml deleted file mode 100644 index 2898df58..00000000 --- a/tools/tests/tester/fixtures/empty_with_description.yaml +++ /dev/null @@ -1,5 +0,0 @@ -$schema: ../json_schemas/test_story.schema.yaml - -description: A story with no beginning or end. - -chapters: [] \ No newline at end of file diff --git a/tools/tests/tester/fixtures/fail_non_existent_variable.yaml b/tools/tests/tester/fixtures/fail_non_existent_variable.yaml index 77e3b8f9..347fdd9e 100644 --- a/tools/tests/tester/fixtures/fail_non_existent_variable.yaml +++ b/tools/tests/tester/fixtures/fail_non_existent_variable.yaml @@ -1,4 +1,4 @@ -$schema: ../json_schemas/test_story.schema.yaml +$schema: ../../../../json_schemas/test_story.schema.yaml description: | This test story checks that we can create a model group diff --git a/tools/tests/tester/fixtures/invalid_story.yaml b/tools/tests/tester/fixtures/invalid_story.yaml new file mode 100644 index 00000000..b59730cd --- /dev/null +++ b/tools/tests/tester/fixtures/invalid_story.yaml @@ -0,0 +1,14 @@ +$schema: ../../../../json_schemas/test_story.schema.yaml + +description: This story is invalid when validated against test_story.schema.yaml. +chapters: + - synopsis: Missing Method + path: /{index} + - synopsis: Invalid Method + path: / + method: RETRIEVE +epilogues: + - path: /{index} + method: DELETE + response: # unexpected property + status: 200 diff --git a/tools/tests/tester/fixtures/valid_story.yaml b/tools/tests/tester/fixtures/valid_story.yaml new file mode 100644 index 00000000..5dabe4b6 --- /dev/null +++ b/tools/tests/tester/fixtures/valid_story.yaml @@ -0,0 +1,13 @@ +$schema: ../../../../json_schemas/test_story.schema.yaml + +description: This story file is a valid story file. +chapters: + - synopsis: A PUT method. + path: /{index} + method: PUT + parameters: + index: one +epilogues: + - path: /one + method: DELETE + status: [200, 404] \ No newline at end of file diff --git a/tools/tests/tester/fixtures/wrong_$schema.yaml b/tools/tests/tester/fixtures/wrong_$schema.yaml new file mode 100644 index 00000000..7482979e --- /dev/null +++ b/tools/tests/tester/fixtures/wrong_$schema.yaml @@ -0,0 +1 @@ +$schema: wrong.schema.yaml \ No newline at end of file diff --git a/tools/tests/tester/helpers.ts b/tools/tests/tester/helpers.ts index 65e9dd46..4f59cf74 100644 --- a/tools/tests/tester/helpers.ts +++ b/tools/tests/tester/helpers.ts @@ -22,6 +22,7 @@ import { NoOpResultLogger, type ResultLogger } from 'tester/ResultLogger' import * as process from 'node:process' import SupplementalChapterEvaluator from 'tester/SupplementalChapterEvaluator' import { Logger } from 'Logger' +import StoryValidator from "../../src/tester/StoryValidator"; export function construct_tester_components (spec_path: string): { specification: OpenAPIV3.Document @@ -30,6 +31,7 @@ export function construct_tester_components (spec_path: string): { chapter_reader: ChapterReader schema_validator: SchemaValidator chapter_evaluator: ChapterEvaluator + story_validator: StoryValidator story_evaluator: StoryEvaluator result_logger: ResultLogger test_runner: TestRunner @@ -46,9 +48,10 @@ export function construct_tester_components (spec_path: string): { const schema_validator = new SchemaValidator(specification, logger) const chapter_evaluator = new ChapterEvaluator(operation_locator, chapter_reader, schema_validator) const supplemental_chapter_evaluator = new SupplementalChapterEvaluator(chapter_reader) + const story_validator = new StoryValidator() const story_evaluator = new StoryEvaluator(chapter_evaluator, supplemental_chapter_evaluator) const result_logger = new NoOpResultLogger() - const test_runner = new TestRunner(story_evaluator, result_logger) + const test_runner = new TestRunner(story_validator, story_evaluator, result_logger) return { specification, operation_locator, @@ -56,6 +59,7 @@ export function construct_tester_components (spec_path: string): { chapter_reader, schema_validator, chapter_evaluator, + story_validator, story_evaluator, result_logger, test_runner diff --git a/tools/tests/tester/test.test.ts b/tools/tests/tester/test.test.ts index 7f5fc5a0..bab38536 100644 --- a/tools/tests/tester/test.test.ts +++ b/tools/tests/tester/test.test.ts @@ -39,9 +39,9 @@ test('displays story filename', () => { ) }) -test('displays story description', () => { - expect(spec(['--tests', 'tools/tests/tester/fixtures/empty_with_description.yaml']).stdout).toContain( - `${ansi.green('PASSED ')} ${ansi.cyan(ansi.b('A story with no beginning or end.'))}` +test('invalid story', () => { + expect(spec(['--tests', 'tools/tests/tester/fixtures/invalid_story.yaml']).stdout).toContain( + `${ansi.gray("(Invalid Story: data/epilogues/0 must NOT have unevaluated properties, data/chapters/0 must have required property 'method', data/chapters/1/method must be equal to one of the allowed values)")}` ) }) @@ -129,6 +129,7 @@ test('check_story_variables', () => { epilogues: [] }) expect(check_story_variables({ + $schema: '', description: 'story1', prologues: [ dummy_chapter_request('prologue1', { x: 'payload.x' }) @@ -139,6 +140,7 @@ test('check_story_variables', () => { })).toStrictEqual(undefined) expect(check_story_variables({ + $schema: '', description: 'story1', prologues: [ dummy_chapter_request('prologue1', { x: 'payload.x' }) @@ -154,6 +156,7 @@ test('check_story_variables', () => { ) expect(check_story_variables({ + $schema: '', description: 'story1', prologues: [ dummy_chapter_request('prologue1', { x: 'payload.x' }) @@ -169,6 +172,7 @@ test('check_story_variables', () => { ) expect(check_story_variables({ + $schema: '', description: 'story1', prologues: [ dummy_chapter_request(undefined, { x: 'payload.x' }) @@ -177,12 +181,13 @@ test('check_story_variables', () => { })).toStrictEqual( failed( [ - { title: "GET /path", overall: { result: Result.FAILED, message: 'An episode must have an id to store its output' } } + { title: "GET /path", overall: { result: Result.FAILED, message: 'A chapter must have an id to store its output' } } ] ) ) expect(check_story_variables({ + $schema: '', description: 'story1', prologues: [ dummy_chapter_request('prologue1', { x: 'payload.x' }) @@ -204,12 +209,12 @@ test('--dry-run', () => { expect(s).not.toContain(`${ansi.yellow('SKIPPED')} CHAPTERS`) expect(s).not.toContain(`${ansi.yellow('SKIPPED')} EPILOGUES`) expect(s).not.toContain(`${ansi.yellow('SKIPPED')} PROLOGUES`) - expect(s).toContain(`${ansi.yellow('SKIPPED')} ${ansi.cyan(ansi.b('A story with all its parts.'))} ${ansi.gray('(' + full_path + ')')}\n\n\n`) + expect(s).toContain(`${ansi.yellow('SKIPPED')} ${ansi.cyan(ansi.b('empty_with_all_the_parts.yaml'))} ${ansi.gray('(' + full_path + ')')}`) }) test('--dry-run --verbose', () => { const s = spec(['--dry-run', '--verbose', '--tests', 'tools/tests/tester/fixtures/empty_with_all_the_parts.yaml']).stdout - expect(s).toContain(`${ansi.yellow('SKIPPED')} ${ansi.cyan(ansi.b('A story with all its parts.'))}`) + expect(s).toContain(`${ansi.yellow('SKIPPED')} ${ansi.cyan(ansi.b('empty_with_all_the_parts.yaml'))}`) expect(s).toContain(`${ansi.yellow('SKIPPED')} CHAPTERS`) expect(s).toContain(`${ansi.yellow('SKIPPED')} EPILOGUES`) expect(s).toContain(`${ansi.yellow('SKIPPED')} PROLOGUES`)