Skip to content

Commit

Permalink
Added StoryValidator (#362)
Browse files Browse the repository at this point in the history
Validating stories before running them.

Signed-off-by: Theo Truong <[email protected]>
  • Loading branch information
nhtruong committed Jun 26, 2024
1 parent 35f3a5c commit 6081d1a
Show file tree
Hide file tree
Showing 31 changed files with 198 additions and 69 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 2 additions & 0 deletions eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
11 changes: 4 additions & 7 deletions json_schemas/test_story.schema.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
$schema: http://json-schema.org/draft-07/schema#
$schema: https://json-schema.org/draft/2019-09/schema

type: object
properties:
Expand All @@ -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:
Expand All @@ -34,7 +33,6 @@ definitions:
response:
$ref: '#/definitions/ExpectedResponse'
required: [synopsis]
additionalProperties: false

ReadChapter:
allOf:
Expand All @@ -47,6 +45,7 @@ definitions:
additionalProperties: false

SupplementalChapter:
unevaluatedProperties: false
allOf:
- $ref: '#/definitions/ChapterRequest'
- type: object
Expand All @@ -57,7 +56,6 @@ definitions:
default: [200, 201]
items:
type: integer
additionalProperties: false

ChapterRequest:
type: object
Expand All @@ -79,7 +77,6 @@ definitions:
output:
$ref: '#/definitions/Output'
required: [path, method]
additionalProperties: false

Output:
description: |
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 0 additions & 1 deletion tests/_core/bulk.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
$schema: ../../json_schemas/test_story.schema.yaml

skip: false
description: Test bulk endpoint.
epilogues:
- path: /books,movies
Expand Down
4 changes: 1 addition & 3 deletions tests/_core/search.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
$schema: ../../json_schemas/test_story.schema.yaml

skip: false
description: Test search endpoint.
prologues:
- path: /movies/_doc
Expand All @@ -12,8 +11,7 @@ prologues:
director: Bennett Miller
title: Moneyball
year: 2011
response:
status: 201
status: [201]
epilogues:
- path: /movies
method: DELETE
Expand Down
1 change: 0 additions & 1 deletion tests/cat/index.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
$schema: ../../json_schemas/test_story.schema.yaml

skip: false
description: Test cat endpoints.
chapters:
- synopsis: Cat with a json response.
Expand Down
2 changes: 1 addition & 1 deletion tools/src/linter/components/NamespaceFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export default class NamespaceFile extends FileValidator {
const current_keys = _.keys(p)
sort_by_keys(p as Record<string, any>, 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)
Expand Down
2 changes: 1 addition & 1 deletion tools/src/linter/components/SupersededOperationsFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion tools/src/tester/ChapterEvaluator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion tools/src/tester/ChapterReader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
8 changes: 5 additions & 3 deletions tools/src/tester/ResultLogger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion tools/src/tester/SchemaValidator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)}`)
Expand Down
29 changes: 12 additions & 17 deletions tools/src/tester/StoryEvaluator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand All @@ -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 } }
}
Expand Down
55 changes: 55 additions & 0 deletions tools/src/tester/StoryValidator.ts
Original file line number Diff line number Diff line change
@@ -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}`,
}
}
}
9 changes: 6 additions & 3 deletions tools/src/tester/TestRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
Expand Down
4 changes: 3 additions & 1 deletion tools/src/tester/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.')
Expand Down Expand Up @@ -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(
Expand Down
9 changes: 7 additions & 2 deletions tools/src/tester/types/eval.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,21 @@

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
display_path: string
full_path: string
description: string
message?: string
chapters: ChapterEvaluation[]
chapters?: ChapterEvaluation[]
epilogues?: ChapterEvaluation[]
prologues?: ChapterEvaluation[]
}
Expand Down
5 changes: 1 addition & 4 deletions tools/src/tester/types/story.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
}
/**
Expand Down
Loading

0 comments on commit 6081d1a

Please sign in to comment.