From 53bdbd93524d73378b307088b35e40b3844a8c90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Vil=C3=A1?= Date: Tue, 11 Jun 2024 11:04:28 -0500 Subject: [PATCH] Support outputs on tests (#324) * Support outputs in tests Signed-off-by: miguel-vila --- CHANGELOG.md | 1 + json_schemas/test_story.schema.yaml | 18 +- spec/namespaces/ml.yaml | 270 ++++++++++++++++++ spec/schemas/ml._common.yaml | 47 +++ tests/ml/model_lifecycle.yaml | 38 +++ tools/src/tester/ChapterEvaluator.ts | 22 +- tools/src/tester/ChapterOutput.ts | 61 ++++ tools/src/tester/ChapterReader.ts | 9 +- tools/src/tester/ResultLogger.ts | 4 +- tools/src/tester/StoryEvaluator.ts | 155 ++++++++-- tools/src/tester/StoryOutputs.ts | 81 ++++++ .../tester/SupplementalChapterEvaluator.ts | 59 ++++ tools/src/tester/helpers.ts | 2 +- tools/src/tester/start.ts | 4 +- tools/src/tester/types/eval.types.ts | 34 +++ tools/src/tester/types/story.types.ts | 20 ++ .../fixtures/fail_non_existent_variable.yaml | 23 ++ tools/tests/tester/helpers.ts | 4 +- tools/tests/tester/start.test.ts | 154 ++++++++++ tools/tests/tester/story_outputs.test.ts | 71 +++++ 20 files changed, 1040 insertions(+), 37 deletions(-) create mode 100644 spec/namespaces/ml.yaml create mode 100644 spec/schemas/ml._common.yaml create mode 100644 tests/ml/model_lifecycle.yaml create mode 100644 tools/src/tester/ChapterOutput.ts create mode 100644 tools/src/tester/StoryOutputs.ts create mode 100644 tools/src/tester/SupplementalChapterEvaluator.ts create mode 100644 tools/tests/tester/fixtures/fail_non_existent_variable.yaml create mode 100644 tools/tests/tester/story_outputs.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index ceae883de..01d39f9ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Added code coverage to tools' tests ([#323](https://github.com/opensearch-project/opensearch-api-specification/pull/323)) - Added a YAML linter ([#312](https://github.com/opensearch-project/opensearch-api-specification/pull/312)) - Added linter to validate order of spec operations ([#325](https://github.com/opensearch-project/opensearch-api-specification/pull/326)) ([#326](https://github.com/opensearch-project/opensearch-api-specification/pull/326)) +- Added support to read outputs from requests in tests([#324](https://github.com/opensearch-project/opensearch-api-specification/pull/324)) ### Changed diff --git a/json_schemas/test_story.schema.yaml b/json_schemas/test_story.schema.yaml index c4fc10a20..126426ff0 100644 --- a/json_schemas/test_story.schema.yaml +++ b/json_schemas/test_story.schema.yaml @@ -66,6 +66,9 @@ definitions: ChapterRequest: type: object properties: + id: + type: string + description: A unique identifier for the chapter, useful for accessing outputs. path: type: string method: @@ -77,9 +80,22 @@ definitions: $ref: '#/definitions/Parameter' request_body: $ref: '#/definitions/RequestBody' + output: + $ref: '#/definitions/Output' required: [path, method] additionalProperties: false + Output: + description: | + Describes output for a chapter. + The keys are the names for the variable in the chapter output. + The values are paths to the values in the response. + The values should be in the form: + - `payload.` for the payload + - `headers.` for the headers + type: object + additionalProperties: + type: string RequestBody: type: object @@ -143,4 +159,4 @@ definitions: - type: boolean - type: string - type: number - - type: boolean \ No newline at end of file + - type: boolean diff --git a/spec/namespaces/ml.yaml b/spec/namespaces/ml.yaml new file mode 100644 index 000000000..4e7b09c56 --- /dev/null +++ b/spec/namespaces/ml.yaml @@ -0,0 +1,270 @@ +openapi: 3.1.0 +info: + title: OpenSearch ML API + description: OpenSearch ML API + version: 1.0.0 +paths: + /_plugins/_ml/model_groups/_register: + post: + operationId: ml.register_model_group.0 + x-operation-group: ml.register_model_group + description: Registers a model group. + requestBody: + $ref: '#/components/requestBodies/ml.register_model_group' + responses: + '200': + $ref: '#/components/responses/ml.register_model_group@200' + /_plugins/_ml/model_groups/{model_group_id}: + get: + operationId: ml.get_model_group.0 + x-operation-group: ml.get_model_group + description: Retrieves a model group. + parameters: + - $ref: '#/components/parameters/ml.get_model_group::path.model_group_id' + responses: + '200': + $ref: '#/components/responses/ml.get_model_group@200' + delete: + operationId: ml.delete_model_group.0 + x-operation-group: ml.delete_model_group + description: Deletes a model group. + parameters: + - $ref: '#/components/parameters/ml.delete_model_group::path.model_group_id' + responses: + '200': + $ref: '#/components/responses/ml.delete_model_group@200' + /_plugins/_ml/models/_register: + post: + operationId: ml.register_model.0 + x-operation-group: ml.register_model + description: Registers a model. + requestBody: + $ref: '#/components/requestBodies/ml.register_model' + responses: + '200': + $ref: '#/components/responses/ml.register_model@200' + /_plugins/_ml/models/{model_id}: + delete: + operationId: ml.delete_model.0 + x-operation-group: ml.delete_model + description: Deletes a model. + parameters: + - $ref: '#/components/parameters/ml.delete_model::path.model_id' + responses: + '200': + $ref: '#/components/responses/ml.delete_model@200' + /_plugins/_ml/tasks/{task_id}: + get: + operationId: ml.get_task.0 + x-operation-group: ml.get_task + description: Retrieves a task. + parameters: + - $ref: '#/components/parameters/ml.get_task::path.task_id' + responses: + '200': + $ref: '#/components/responses/ml.get_task@200' + /_plugins/_ml/models/_search: + get: + operationId: ml.search_models.0 + x-operation-group: ml.search_models + description: Searches for models. + requestBody: + $ref: '#/components/requestBodies/ml.search_models' + responses: + '200': + $ref: '#/components/responses/ml.search_models@200' +components: + requestBodies: + ml.register_model_group: + content: + application/json: + schema: + type: object + properties: + name: + type: string + description: The model group name. + description: + type: string + description: The model group description. + access_mode: + type: string + description: The model group access mode. + enum: [private, public, restricted] + backend_roles: + type: array + items: + type: string + description: The backend roles. + add_all_backend_roles: + type: boolean + description: The add all backend roles. + required: + - name + ml.register_model: + content: + application/json: + schema: + type: object + properties: + name: + type: string + description: The model name. + version: + type: string + description: The model version. + model_format: + type: string + description: The portable format of the model file. + enum: [TORCH_SCRIPT, ONNX] + description: + type: string + description: The model description. + model_group_id: + type: string + description: The ID of the model group to which to register the model. + required: + - name + - version + - model_format + ml.search_models: + content: + application/json: + schema: + type: object + properties: + query: + type: object + # TODO: Define the query schema + description: The query. + size: + type: integer + description: The number of models to return. + required: + - query + - size + responses: + ml.register_model_group@200: + content: + application/json: + schema: + type: object + properties: + model_group_id: + type: string + description: The model group ID. + status: + type: string + description: The status. + required: + - model_group_id + - status + ml.get_model_group@200: + content: + application/json: + schema: + type: object + properties: + name: + type: string + description: The model group name. + latest_version: + type: number + description: The latest version. + description: + type: string + description: The model group description. + access: + type: string + description: The model group access. + required: + - name + - latest_version + - description + - access + ml.delete_model_group@200: + content: + application/json: + schema: + type: object + ml.register_model@200: + content: + application/json: + schema: + type: object + properties: + task_id: + type: string + description: The task ID. + model_id: + type: string + description: The model ID. + status: + type: string + description: The status. + required: + # model_id doesn't seem to be required in the response + # it can be eventually retrieved through the task_id + - task_id + - status + ml.delete_model@200: + content: + application/json: + schema: + type: object + ml.get_task@200: + content: + application/json: + schema: + type: object + properties: + model_id: + type: string + description: The model ID. + state: + type: string + description: The state. + enum: + - CREATED + - RUNNING + - COMPLETED + - FAILED + - CANCELLED + - COMPLETED_WITH_ERROR + required: + - state + ml.search_models@200: + content: + application/json: + schema: + type: object + properties: + hits: + $ref: '../schemas/ml._common.yaml#/components/schemas/SearchModelHits' + required: + - hits + parameters: + ml.get_model_group::path.model_group_id: + name: model_group_id + in: path + required: true + schema: + type: string + ml.delete_model_group::path.model_group_id: + name: model_group_id + in: path + required: true + schema: + type: string + ml.delete_model::path.model_id: + name: model_id + in: path + required: true + schema: + type: string + ml.get_task::path.task_id: + name: task_id + in: path + required: true + schema: + type: string diff --git a/spec/schemas/ml._common.yaml b/spec/schemas/ml._common.yaml new file mode 100644 index 000000000..fa2c3f998 --- /dev/null +++ b/spec/schemas/ml._common.yaml @@ -0,0 +1,47 @@ +openapi: 3.1.0 +info: + title: Schemas of ml category + description: Schemas of ml category + version: 1.0.0 +paths: {} +components: + schemas: + SearchModelHits: + type: object + properties: + total: + $ref: '#/components/schemas/HitsTotal' + hits: + type: array + items: + $ref: '#/components/schemas/SearchModelHitsHit' + required: + - total + - hits + HitsTotal: + type: object + properties: + value: + type: integer + description: The total number of hits. + relation: + type: string + description: The relation. + required: + - value + - relation + SearchModelHitsHit: + type: object + properties: + _index: + type: string + description: The index. + _id: + type: string + description: The hit ID. + model_id: + type: string + description: The model ID. + required: + - _id + - model_id diff --git a/tests/ml/model_lifecycle.yaml b/tests/ml/model_lifecycle.yaml new file mode 100644 index 000000000..ddbc13537 --- /dev/null +++ b/tests/ml/model_lifecycle.yaml @@ -0,0 +1,38 @@ +$schema: ../json_schemas/test_story.schema.yaml + +skip: false +description: | + This test story checks that we can create a model group +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 + method: PUT + request_body: + payload: + persistent: + plugins: + ml_commons: + only_run_on_ml_node: false + - synopsis: Create model group + id: create_model_group + path: /_plugins/_ml/model_groups/_register + method: POST + request_body: + payload: + name: "NLP_Group" + description: "Model group for NLP models" + output: + test_model_group_id: "payload.model_group_id" + - synopsis: Query model group + path: /_plugins/_ml/model_groups/{model_group_id} + method: GET + parameters: + model_group_id: ${create_model_group.test_model_group_id} + response: + status: 200 diff --git a/tools/src/tester/ChapterEvaluator.ts b/tools/src/tester/ChapterEvaluator.ts index c68b03d8e..fefa0202f 100644 --- a/tools/src/tester/ChapterEvaluator.ts +++ b/tools/src/tester/ChapterEvaluator.ts @@ -14,36 +14,40 @@ import { overall_result } from './helpers' import type ChapterReader from './ChapterReader' import type OperationLocator from './OperationLocator' import type SchemaValidator from './SchemaValidator' +import { type StoryOutputs } from './StoryOutputs' +import { ChapterOutput } from './ChapterOutput' export default class ChapterEvaluator { private readonly _operation_locator: OperationLocator private readonly _chapter_reader: ChapterReader private readonly _schema_validator: SchemaValidator - constructor (spec_parser: OperationLocator, chapter_reader: ChapterReader, schema_validator: SchemaValidator) { + constructor(spec_parser: OperationLocator, chapter_reader: ChapterReader, schema_validator: SchemaValidator) { this._operation_locator = spec_parser this._chapter_reader = chapter_reader this._schema_validator = schema_validator } - async evaluate (chapter: Chapter, skip: boolean): Promise { + async evaluate(chapter: Chapter, skip: boolean, story_outputs: StoryOutputs): Promise { if (skip) return { title: chapter.synopsis, overall: { result: Result.SKIPPED } } - const response = await this._chapter_reader.read(chapter) + const response = await this._chapter_reader.read(chapter, story_outputs) const operation = this._operation_locator.locate_operation(chapter) if (operation == null) return { title: chapter.synopsis, overall: { result: Result.FAILED, message: `Operation "${chapter.method.toUpperCase()} ${chapter.path}" not found in the spec.` } } const params = this.#evaluate_parameters(chapter, operation) const request_body = this.#evaluate_request_body(chapter, operation) const status = this.#evaluate_status(chapter, response) const payload = status.result === Result.PASSED ? this.#evaluate_payload(response, operation) : { result: Result.SKIPPED } + const output_values = ChapterOutput.extract_output_values(response, chapter.output) return { title: chapter.synopsis, - overall: { result: overall_result(Object.values(params).concat([request_body, status, payload])) }, + overall: { result: overall_result(Object.values(params).concat([request_body, status, payload]).concat(output_values ? [output_values] : [])) }, request: { parameters: params, request_body }, - response: { status, payload } + response: { status, payload }, + ...(output_values ? { output_values } : {}) } } - #evaluate_parameters (chapter: Chapter, operation: ParsedOperation): Record { + #evaluate_parameters(chapter: Chapter, operation: ParsedOperation): Record { return Object.fromEntries(Object.entries(chapter.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.` }] @@ -52,7 +56,7 @@ export default class ChapterEvaluator { })) } - #evaluate_request_body (chapter: Chapter, operation: ParsedOperation): Evaluation { + #evaluate_request_body(chapter: Chapter, operation: ParsedOperation): Evaluation { if (!chapter.request_body) return { result: Result.PASSED } const content_type = chapter.request_body.content_type ?? 'application/json' const schema = operation.requestBody?.content[content_type]?.schema @@ -60,7 +64,7 @@ export default class ChapterEvaluator { return this._schema_validator.validate(schema, chapter.request_body?.payload ?? {}) } - #evaluate_status (chapter: Chapter, response: ActualResponse): Evaluation { + #evaluate_status(chapter: Chapter, response: ActualResponse): Evaluation { const expected_status = chapter.response?.status ?? 200 if (response.status === expected_status) return { result: Result.PASSED } return { @@ -70,7 +74,7 @@ export default class ChapterEvaluator { } } - #evaluate_payload (response: ActualResponse, operation: ParsedOperation): Evaluation { + #evaluate_payload(response: ActualResponse, operation: ParsedOperation): Evaluation { const content_type = response.content_type ?? 'application/json' const content = operation.responses[response.status]?.content[content_type] const schema = content?.schema diff --git a/tools/src/tester/ChapterOutput.ts b/tools/src/tester/ChapterOutput.ts new file mode 100644 index 000000000..c58a4ec0e --- /dev/null +++ b/tools/src/tester/ChapterOutput.ts @@ -0,0 +1,61 @@ +/* +* 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 { EvaluationWithOutput, Result } from './types/eval.types' +import { ActualResponse, type Output } from './types/story.types' +import _ from 'lodash' + +export class ChapterOutput { + private outputs: Record + constructor (outputs: Record) { + this.outputs = outputs + } + + get_output (name: string): any { + return this.outputs[name] + } + + set_output (name: string, value: any): void { + this.outputs[name] = value + } + + /** + * Creates a dummy ChapterOutput from an Output object + * where the values will be the response paths. + * Used for a validation check. + * @param output + * @returns + */ + static create_dummy_from_output (output: Output): ChapterOutput { + return new ChapterOutput(output) + } + + static extract_output_values (response: ActualResponse, output?: Output): EvaluationWithOutput | undefined { + if (!output) return undefined + const chapter_output = new ChapterOutput({}) + for (const [name, path] of Object.entries(output)) { + const [source, ...rest] = path.split('.') + const keys = rest.join('.') + let value: any + if (source === 'payload') { + if (response.payload === undefined) { + return { result: Result.ERROR, message: 'No payload found in response, but expected output: ' + path } + } + value = keys.length === 0 ? response.payload : _.get(response.payload, keys) + if (value === undefined) { + return { result: Result.ERROR, message: `Expected to find non undefined value at \`${path}\`.` } + } + } else { + return { result: Result.ERROR, message: 'Unknown output source: ' + source } + } + chapter_output.set_output(name, value) + } + return { result: Result.PASSED, output: chapter_output } + } +} diff --git a/tools/src/tester/ChapterReader.ts b/tools/src/tester/ChapterReader.ts index bd5774011..fa976b113 100644 --- a/tools/src/tester/ChapterReader.ts +++ b/tools/src/tester/ChapterReader.ts @@ -9,6 +9,7 @@ import { type ChapterRequest, type ActualResponse, type Parameter } from './types/story.types' import { type OpenSearchHttpClient } from '../OpenSearchHttpClient' +import { type StoryOutputs } from './StoryOutputs' // A lightweight client for testing the API export default class ChapterReader { @@ -18,14 +19,16 @@ export default class ChapterReader { this._client = client } - async read (chapter: ChapterRequest): Promise { + async read (chapter: ChapterRequest, story_outputs: StoryOutputs): Promise { const response: Record = {} - const [url_path, params] = this.#parse_url(chapter.path, chapter.parameters ?? {}) + const resolved_params = story_outputs.resolve_params(chapter.parameters ?? {}) + const [url_path, params] = this.#parse_url(chapter.path, resolved_params) + const request_data = chapter.request_body?.payload !== undefined ? story_outputs.resolve_value(chapter.request_body.payload) : undefined await this._client.request({ url: url_path, method: chapter.method, params, - data: chapter.request_body?.payload + data: request_data }).then(r => { response.status = r.status response.content_type = r.headers['content-type'].split(';')[0] diff --git a/tools/src/tester/ResultLogger.ts b/tools/src/tester/ResultLogger.ts index 086387e81..a364263fc 100644 --- a/tools/src/tester/ResultLogger.ts +++ b/tools/src/tester/ResultLogger.ts @@ -36,8 +36,8 @@ export class ConsoleResultLogger implements ResultLogger { console.log('\n') } - #log_story ({ result, full_path, description, display_path }: StoryEvaluation): void { - this.#log_evaluation({ result, message: full_path }, ansi.cyan(ansi.b(description ?? display_path))) + #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_chapters (evaluations: ChapterEvaluation[], title: string): void { diff --git a/tools/src/tester/StoryEvaluator.ts b/tools/src/tester/StoryEvaluator.ts index 9769195ac..77d26d347 100644 --- a/tools/src/tester/StoryEvaluator.ts +++ b/tools/src/tester/StoryEvaluator.ts @@ -7,11 +7,13 @@ * compatible open source license. */ -import { type Chapter, type Story, type SupplementalChapter } from './types/story.types' -import { type ChapterEvaluation, Result, type StoryEvaluation } from './types/eval.types' +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 ChapterEvaluator from './ChapterEvaluator' -import type ChapterReader from './ChapterReader' import { overall_result } from './helpers' +import { StoryOutputs } from './StoryOutputs' +import SupplementalChapterEvaluator from './SupplementalChapterEvaluator' +import { ChapterOutput } from './ChapterOutput' export interface StoryFile { display_path: string @@ -20,15 +22,15 @@ export interface StoryFile { } export default class StoryEvaluator { - private readonly _chapter_reader: ChapterReader private readonly _chapter_evaluator: ChapterEvaluator + private readonly _supplemental_chapter_evaluator: SupplementalChapterEvaluator - constructor (chapter_reader: ChapterReader, chapter_evaluator: ChapterEvaluator) { - this._chapter_reader = chapter_reader + constructor(chapter_evaluator: ChapterEvaluator, supplemental_chapter_evaluator: SupplementalChapterEvaluator) { this._chapter_evaluator = chapter_evaluator + this._supplemental_chapter_evaluator = supplemental_chapter_evaluator } - async evaluate ({ story, display_path, full_path }: StoryFile, dry_run: boolean = false): Promise { + async evaluate({ story, display_path, full_path }: StoryFile, dry_run: boolean = false): Promise { if (story.skip) { return { result: Result.SKIPPED, @@ -38,9 +40,14 @@ export default class StoryEvaluator { chapters: [] } } - const { evaluations: prologues, has_errors: prologue_errors } = await this.#evaluate_supplemental_chapters(story.prologues ?? [], dry_run) - const chapters = await this.#evaluate_chapters(story.chapters, prologue_errors, dry_run) - const { evaluations: epilogues } = await this.#evaluate_supplemental_chapters(story.epilogues ?? [], dry_run) + const variables_error = StoryEvaluator.check_story_variables(story, display_path, full_path ) + if (variables_error !== undefined) { + return variables_error + } + const story_outputs = new StoryOutputs() + const { evaluations: prologues, has_errors: prologue_errors } = await this.#evaluate_supplemental_chapters(story.prologues ?? [], dry_run, story_outputs) + const chapters = await this.#evaluate_chapters(story.chapters, prologue_errors, dry_run, story_outputs) + const { evaluations: epilogues } = await this.#evaluate_supplemental_chapters(story.epilogues ?? [], dry_run, story_outputs) return { display_path, full_path, @@ -52,22 +59,25 @@ export default class StoryEvaluator { } } - async #evaluate_chapters (chapters: Chapter[], has_errors: boolean, dry_run: boolean): Promise { + async #evaluate_chapters(chapters: Chapter[], has_errors: boolean, dry_run: boolean, story_outputs: StoryOutputs): Promise { const evaluations: ChapterEvaluation[] = [] for (const chapter of chapters) { if (dry_run) { const title = chapter.synopsis || `${chapter.method} ${chapter.path}` evaluations.push({ title, overall: { result: Result.SKIPPED, message: 'Dry Run', error: undefined } }) } else { - const evaluation = await this._chapter_evaluator.evaluate(chapter, has_errors) + const evaluation = await this._chapter_evaluator.evaluate(chapter, has_errors, story_outputs) has_errors = has_errors || evaluation.overall.result === Result.ERROR + if (evaluation.output_values?.output !== undefined && chapter.id !== undefined) { + story_outputs.set_chapter_output(chapter.id, evaluation.output_values?.output) + } evaluations.push(evaluation) } } return evaluations } - async #evaluate_supplemental_chapters (chapters: SupplementalChapter[], dry_run: boolean): Promise<{ evaluations: ChapterEvaluation[], has_errors: boolean }> { + async #evaluate_supplemental_chapters(chapters: SupplementalChapter[], dry_run: boolean, story_outputs: StoryOutputs): Promise<{ evaluations: ChapterEvaluation[], has_errors: boolean }> { let has_errors = false const evaluations: ChapterEvaluation[] = [] for (const chapter of chapters) { @@ -75,15 +85,122 @@ export default class StoryEvaluator { if (dry_run) { evaluations.push({ title, overall: { result: Result.SKIPPED, message: 'Dry Run', error: undefined } }) } else { - const response = await this._chapter_reader.read(chapter) - const status = chapter.status ?? [200, 201] - if (status.includes(response.status)) evaluations.push({ title, overall: { result: Result.PASSED } }) - else { - has_errors = true - evaluations.push({ title, overall: { result: Result.ERROR, message: response.message, error: response.error as Error } }) + const { evaluation, evaluation_error } = await this._supplemental_chapter_evaluator.evaluate(chapter, story_outputs) + has_errors = has_errors || evaluation_error + if (evaluation.output_values?.output !== undefined && chapter.id !== undefined) { + story_outputs.set_chapter_output(chapter.id, evaluation.output_values?.output) } + evaluations.push(evaluation) } } return { evaluations, has_errors } } + + 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) + }) + const chapters = (story.chapters ?? []).map((chapter) => { + return StoryEvaluator.#check_episode_variables(chapter, story_outputs) + }) + const epilogues = (story.epilogues ?? []).map((epilogue) => { + return StoryEvaluator.#check_episode_variables(epilogue, story_outputs) + }) + const evals = prologues.concat(chapters).concat(epilogues) + if (overall_result(evals.map(e => e.overall)) === Result.FAILED) { + return { + result: Result.ERROR, + display_path, + full_path, + description: story.description, + prologues, + chapters, + epilogues, + message: 'The story was defined with incorrect variables' + } + } + } + + 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) + 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 (episode.id !== undefined && episode.output !== undefined) { + story_outputs.set_chapter_output(episode.id, ChapterOutput.create_dummy_from_output(episode.output)) + } + return { title, overall: { result: Result.PASSED } } + } + + /** + * + * @param chapter ChapterEvaluation { + title: string + overall: Evaluation + * @param story_outputs + * @returns + */ + static #check_used_variables(chapter: ChapterRequest, story_outputs: StoryOutputs): ChapterEvaluation | undefined { + const variables = new Set() + const title = `${chapter.method} ${chapter.path}` + StoryEvaluator.#extract_params_variables(chapter.parameters ?? {}, variables) + StoryEvaluator.#extract_request_body_variables(chapter.request_body?.payload ?? {}, variables) + for (const { chapter_id, output_name } of variables) { + if (!story_outputs.has_chapter(chapter_id)) { + return StoryEvaluator.#failed_evaluation(title, `Chapter makes reference to non existent chapter "${chapter_id}`) + } + if (!story_outputs.has_output_value(chapter_id, output_name)) { + return StoryEvaluator.#failed_evaluation(title, `Chapter makes reference to non existent output "${output_name}" in chapter "${chapter_id}"`) + } + } + } + + static #extract_params_variables(parameters: Record, variables: Set): void { + Object.values(parameters ?? {}).forEach((param) => { + if (typeof param === 'string') { + const ref = OutputReference.parse(param) + if (ref) { + variables.add(ref) + } + } + }) + } + + static #extract_request_body_variables(request_body: any, variables: Set): void { + const request_body_type = typeof request_body + switch (request_body_type) { + case 'string': { + const ref = OutputReference.parse(request_body as string) + if (ref !== undefined) { + variables.add(ref) + } + break + } + case 'object': { + if (Array.isArray(request_body)) { + for (const value of request_body) { + StoryEvaluator.#extract_request_body_variables(value, variables) + } + } else { + for (const [, value] of Object.entries(request_body as Record)) { + StoryEvaluator.#extract_request_body_variables(value, variables) + } + } + break + } + default: { + break + } + } + } + + static #failed_evaluation(title: string, message: string): ChapterEvaluation { + return { title, overall: { result: Result.FAILED, message } } + } + } diff --git a/tools/src/tester/StoryOutputs.ts b/tools/src/tester/StoryOutputs.ts new file mode 100644 index 000000000..37a936dd9 --- /dev/null +++ b/tools/src/tester/StoryOutputs.ts @@ -0,0 +1,81 @@ +/* +* 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 { type ChapterOutput } from './ChapterOutput' +import { OutputReference } from './types/eval.types' +import { type Parameter } from './types/story.types' + +export class StoryOutputs { + private outputs: Record + + constructor (outputs: Record = {}) { + this.outputs = outputs + } + + has_chapter (chapter_id: string): boolean { + return this.outputs[chapter_id] !== undefined + } + + has_output_value (chapter_id: string, output_name: string): boolean { + return this.has_chapter(chapter_id) && this.outputs[chapter_id].get_output(output_name) !== undefined + } + + set_chapter_output (chapter_id: string, output: ChapterOutput): void { + this.outputs[chapter_id] = output + } + + get_output_value (chapter_id: string, output_name: string): any { + return this.outputs[chapter_id].get_output(output_name) + } + + resolve_params (parameters: Record): Record { + const resolved_params: Record = {} + for (const [param_name, param_value] of Object.entries(parameters ?? {})) { + if (typeof param_value === 'string') { + resolved_params[param_name] = this.resolve_string(param_value) + } else { + resolved_params[param_name] = param_value + } + } + return resolved_params + } + + resolve_string (str: string): any { + const ref = OutputReference.parse(str) + if (ref) { + return this.get_output_value(ref.chapter_id, ref.output_name) + } else { + return str + } + } + + resolve_value (payload: any): any { + const payload_type = typeof payload + switch (payload_type) { + case 'string': + return this.resolve_string(payload as string) + case 'object': + if (Array.isArray(payload)) { + const resolved_array: any[] = [] + for (const value of payload) { + resolved_array.push(this.resolve_value(value)) + } + return resolved_array + } else { + const resolved_obj: Record = {} + for (const [key, value] of Object.entries(payload as Record)) { + resolved_obj[key] = this.resolve_value(value) + } + return resolved_obj + } + default: + return payload + } + } +} diff --git a/tools/src/tester/SupplementalChapterEvaluator.ts b/tools/src/tester/SupplementalChapterEvaluator.ts new file mode 100644 index 000000000..34d9972fd --- /dev/null +++ b/tools/src/tester/SupplementalChapterEvaluator.ts @@ -0,0 +1,59 @@ +/* +* 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 { ChapterOutput } from "./ChapterOutput"; +import ChapterReader from "./ChapterReader"; +import { StoryOutputs } from "./StoryOutputs"; +import { overall_result } from "./helpers"; +import { ChapterEvaluation, Result } from "./types/eval.types"; +import { SupplementalChapter } from "./types/story.types"; + +export default class SupplementalChapterEvaluator { + private readonly _chapter_reader: ChapterReader; + + constructor(chapter_reader: ChapterReader) { + this._chapter_reader = chapter_reader; + } + + async evaluate(chapter: SupplementalChapter, story_outputs: StoryOutputs): Promise<{ evaluation: ChapterEvaluation, evaluation_error: boolean }> { + const title = `${chapter.method} ${chapter.path}` + const response = await this._chapter_reader.read(chapter, story_outputs) + const status = chapter.status ?? [200, 201] + const output_values = ChapterOutput.extract_output_values(response, chapter.output) + let response_evaluation: ChapterEvaluation + const passed_evaluation = { title, overall: { result: Result.PASSED } } + if (status.includes(response.status)) { + response_evaluation = passed_evaluation + } else { + response_evaluation = { title, overall: { result: Result.ERROR, message: response.message, error: response.error as Error }, output_values } + } + if (output_values) { + response_evaluation.output_values = output_values + } + const result = overall_result([response_evaluation.overall].concat(output_values ? [output_values] : [])) + if (result === Result.PASSED) { + return { evaluation: passed_evaluation, evaluation_error: false } + } else { + const message_segments = [] + if (response_evaluation.overall.result === Result.ERROR) { + message_segments.push(`${response_evaluation.overall.message}`) + } + if (output_values !== undefined && output_values.result === Result.ERROR) { + message_segments.push(`${output_values.message}`) + } + const message = message_segments.join('\n') + const evaluation = { + title, + overall: { result: Result.ERROR, message, error: response.error as Error }, + ...(output_values ? { output_values } : {}) + } + return { evaluation, evaluation_error: true } + } + } +} diff --git a/tools/src/tester/helpers.ts b/tools/src/tester/helpers.ts index 4dd7abe66..684a57e27 100644 --- a/tools/src/tester/helpers.ts +++ b/tools/src/tester/helpers.ts @@ -9,7 +9,7 @@ import { type Evaluation, Result } from './types/eval.types' -export function overall_result (evaluations: Evaluation[]): Result { +export function overall_result(evaluations: Evaluation[]): Result { if (evaluations.some(e => e.result === Result.ERROR)) return Result.ERROR if (evaluations.some(e => e.result === Result.FAILED)) return Result.FAILED if (evaluations.some(e => e.result === Result.SKIPPED)) return Result.SKIPPED diff --git a/tools/src/tester/start.ts b/tools/src/tester/start.ts index b99d40201..0d002df95 100644 --- a/tools/src/tester/start.ts +++ b/tools/src/tester/start.ts @@ -25,6 +25,7 @@ import SchemaValidator from './SchemaValidator' import StoryEvaluator from './StoryEvaluator' import { ConsoleResultLogger } from './ResultLogger' import * as process from 'node:process' +import SupplementalChapterEvaluator from './SupplementalChapterEvaluator' const command = new Command() .description('Run test stories against the OpenSearch spec.') @@ -52,7 +53,8 @@ const spec = (new OpenApiMerger(opts.specPath, logger)).merge() const http_client = new OpenSearchHttpClient(get_opensearch_opts_from_cli(opts)) const chapter_reader = new ChapterReader(http_client) const chapter_evaluator = new ChapterEvaluator(new OperationLocator(spec), chapter_reader, new SchemaValidator(spec)) -const story_evaluator = new StoryEvaluator(chapter_reader, chapter_evaluator) +const supplemental_chapter_evaluator = new SupplementalChapterEvaluator(chapter_reader) +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) diff --git a/tools/src/tester/types/eval.types.ts b/tools/src/tester/types/eval.types.ts index 166d72c07..e4c4fb160 100644 --- a/tools/src/tester/types/eval.types.ts +++ b/tools/src/tester/types/eval.types.ts @@ -7,6 +7,9 @@ * compatible open source license. */ +import { type ChapterOutput } from '../ChapterOutput' +import { StoryOutputs } from '../StoryOutputs' + export type LibraryEvaluation = StoryEvaluation[] export interface StoryEvaluation { @@ -31,6 +34,16 @@ export interface ChapterEvaluation { status: Evaluation payload: Evaluation } + output_values?: EvaluationWithOutput +} + +export class ChaptersEvaluations { + evaluations: ChapterEvaluation[] + outputs: StoryOutputs + constructor () { + this.evaluations = [] + this.outputs = new StoryOutputs() + } } export interface Evaluation { @@ -39,9 +52,30 @@ export interface Evaluation { error?: Error | string } +export type EvaluationWithOutput = Evaluation & { + output?: ChapterOutput +} + export enum Result { PASSED = 'PASSED', FAILED = 'FAILED', SKIPPED = 'SKIPPED', ERROR = 'ERROR', } + +export class OutputReference { + chapter_id: string + output_name: string + private constructor (chapter_id: string, output_name: string) { + this.chapter_id = chapter_id + this.output_name = output_name + } + + static parse (str: string): OutputReference | undefined { + if (str.startsWith('${') && str.endsWith('}')) { + const spl = str.slice(2, -1).split('.', 2) + return { chapter_id: spl[0], output_name: spl[1] } + } + return undefined + } +} diff --git a/tools/src/tester/types/story.types.ts b/tools/src/tester/types/story.types.ts index 344a15880..c7d52c3d1 100644 --- a/tools/src/tester/types/story.types.ts +++ b/tools/src/tester/types/story.types.ts @@ -65,12 +65,17 @@ export interface Story { * via the `definition` "ChapterRequest". */ export interface ChapterRequest { + /** + * A unique identifier for the chapter, useful for accessing outputs. + */ + id?: string; path: string; method: 'GET' | 'PUT' | 'POST' | 'DELETE' | 'PATCH' | 'HEAD' | 'OPTIONS'; parameters?: { [k: string]: Parameter; }; request_body?: RequestBody; + output?: Output; } /** * This interface was referenced by `Story`'s JSON-Schema @@ -80,6 +85,21 @@ export interface RequestBody { content_type?: string; payload: Payload; } +/** + * Describes output for a chapter. + * The keys are the names for the variable in the chapter output. + * The values are paths to the values in the response. + * The values should be in the form: + * - `payload.` for the payload + * - `headers.` for the headers + * + * + * This interface was referenced by `Story`'s JSON-Schema + * via the `definition` "Output". + */ +export interface Output { + [k: string]: string; +} /** * This interface was referenced by `Story`'s JSON-Schema * via the `definition` "ExpectedResponse". diff --git a/tools/tests/tester/fixtures/fail_non_existent_variable.yaml b/tools/tests/tester/fixtures/fail_non_existent_variable.yaml new file mode 100644 index 000000000..d1f50797a --- /dev/null +++ b/tools/tests/tester/fixtures/fail_non_existent_variable.yaml @@ -0,0 +1,23 @@ +$schema: ../json_schemas/test_story.schema.yaml + +skip: false +description: | + This test story checks that we can create a model group +chapters: + - synopsis: Create model group + id: create_model_group + path: /_plugins/_ml/model_groups/_register + method: POST + request_body: + payload: + name: "NLP_Group" + description: "Model group for NLP models" + output: + test_model_group_id: "payload.model_group_id" + - synopsis: Query model group + path: /_plugins/_ml/model_groups/{model_group_id} + method: GET + parameters: + model_group_id: ${create_model_group.test_model_group_id_typo} + response: + status: 200 diff --git a/tools/tests/tester/helpers.ts b/tools/tests/tester/helpers.ts index 9bb984458..86312f80c 100644 --- a/tools/tests/tester/helpers.ts +++ b/tools/tests/tester/helpers.ts @@ -20,6 +20,7 @@ import { type OpenAPIV3 } from 'openapi-types' import TestRunner from 'tester/TestRunner' import { NoOpResultLogger, type ResultLogger } from 'tester/ResultLogger' import * as process from 'node:process' +import SupplementalChapterEvaluator from 'tester/SupplementalChapterEvaluator' export function construct_tester_components (spec_path: string): { specification: OpenAPIV3.Document @@ -42,7 +43,8 @@ export function construct_tester_components (spec_path: string): { const chapter_reader = new ChapterReader(opensearch_http_client) const schema_validator = new SchemaValidator(specification) const chapter_evaluator = new ChapterEvaluator(operation_locator, chapter_reader, schema_validator) - const story_evaluator = new StoryEvaluator(chapter_reader, chapter_evaluator) + const supplemental_chapter_evaluator = new SupplementalChapterEvaluator(chapter_reader) + const story_evaluator = new StoryEvaluator(chapter_evaluator, supplemental_chapter_evaluator) const result_logger = new NoOpResultLogger() const test_runner = new TestRunner(story_evaluator, result_logger) return { diff --git a/tools/tests/tester/start.test.ts b/tools/tests/tester/start.test.ts index 8e6fc88e3..526669f5b 100644 --- a/tools/tests/tester/start.test.ts +++ b/tools/tests/tester/start.test.ts @@ -10,6 +10,10 @@ import { spawnSync } from 'child_process' import * as ansi from 'tester/Ansi' import * as path from 'path' +import { type Chapter, type ChapterRequest, type Output, type RequestBody, type ActualResponse, Story } from 'tester/types/story.types' +import { type EvaluationWithOutput, Result, ChapterEvaluation, StoryEvaluation } from 'tester/types/eval.types' +import { ChapterOutput } from 'tester/ChapterOutput' +import StoryEvaluator from 'tester/StoryEvaluator' const spec = (args: string[]): any => { const start = spawnSync('ts-node', ['tools/src/tester/start.ts'].concat(args), { @@ -41,6 +45,156 @@ test('displays story description', () => { ) }) +function create_response(payload: any): ActualResponse { + return { + status: 200, + content_type: 'application/json', + payload + } +} + +function passed_output(output: Record): EvaluationWithOutput { + return { + result: Result.PASSED, + output: new ChapterOutput(output) + } +} + +test('extract_output_values', () => { + const response: ActualResponse = create_response({ + a: { + b: { + c: 1 + }, + arr: [ + { d: 2 }, + { e: 3 } + ] + } + }) + const output1 = { + c: 'payload.a.b.c', + d: 'payload.a.arr[0].d', + e: 'payload.a.arr[1].e' + } + expect(ChapterOutput.extract_output_values(response, output1)).toEqual(passed_output({ + c: 1, + d: 2, + e: 3 + })) + expect(ChapterOutput.extract_output_values(response, { x: 'payload' })).toEqual( + passed_output({ x: response.payload }) + ) + expect(ChapterOutput.extract_output_values(response, { x: 'payload.a.b.x[0]' })).toEqual({ + result: Result.ERROR, + message: 'Expected to find non undefined value at `payload.a.b.x[0]`.' + }) +}) + +function dummy_chapter_request(id?: string, output?: Output): ChapterRequest { + return { + id, + path: '/path', + method: 'GET', + output + } +} + +function dummy_chapter_request_with_input(parameters?: Record, request_body?: RequestBody, id?: string, output?: Output): ChapterRequest { + return { + ...dummy_chapter_request(id, output), + parameters, + request_body + } +} + +function chapter(synopsis: string, request: ChapterRequest): Chapter { + return { + synopsis, + ...request + } +} + +/* eslint-disable no-template-curly-in-string */ +test('check_story_variables', () => { + const check_story_variables = (s: Story): StoryEvaluation | undefined => StoryEvaluator.check_story_variables(s, 'display_path', 'full_path') + const failed = (prologues: ChapterEvaluation[] = [], chapters: ChapterEvaluation[] = []): StoryEvaluation => ({ + result: Result.ERROR, + description: 'story1', + display_path: 'display_path', + full_path: 'full_path', + message: 'The story was defined with incorrect variables', + prologues, + chapters, + epilogues: [] + }) + expect(check_story_variables({ + description: 'story1', + prologues: [ + dummy_chapter_request('prologue1', { x: 'payload.x' }) + ], + chapters: [ + chapter('synopsis-1', dummy_chapter_request_with_input({ 'param-x': '${prologue1.x}' })) + ] + })).toStrictEqual(undefined) + + expect(check_story_variables({ + description: 'story1', + prologues: [ + dummy_chapter_request('prologue1', { x: 'payload.x' }) + ], + chapters: [ + chapter('synopsis-1', dummy_chapter_request_with_input({ 'param-x': '${prologue1.y}' })) + ] + })).toStrictEqual( + failed( + [{ title: "GET /path", overall: { result: Result.PASSED } }], + [{ title: 'GET /path', overall: { result: Result.FAILED, message: 'Chapter makes reference to non existent output "y" in chapter "prologue1"' } }] + ) + ) + + expect(check_story_variables({ + description: 'story1', + prologues: [ + dummy_chapter_request('prologue1', { x: 'payload.x' }) + ], + chapters: [ + chapter('synopsis-1', dummy_chapter_request_with_input({ 'param-x': '${prologue2.x}' })) + ] + })).toStrictEqual( + failed( + [{ title: "GET /path", overall: { result: Result.PASSED } }], + [{ title: 'GET /path', overall: { result: Result.FAILED, message: 'Chapter makes reference to non existent chapter "prologue2' } }] + ) + ) + + expect(check_story_variables({ + description: 'story1', + prologues: [ + dummy_chapter_request(undefined, { x: 'payload.x' }) + ], + chapters: [] + })).toStrictEqual( + failed( + [ + { title: "GET /path", overall: { result: Result.FAILED, message: 'An episode must have an id to store its output' } } + ] + ) + ) + + expect(check_story_variables({ + description: 'story1', + prologues: [ + dummy_chapter_request('prologue1', { x: 'payload.x' }) + ], + chapters: [ + chapter('synopsis-1', dummy_chapter_request_with_input({ 'param-x': '${prologue1.x}' }, undefined, 'chapter1', { y: 'payload.y' })), + chapter('synopsis-2', dummy_chapter_request_with_input({ 'param-y': '${chapter1.y}' })) + ] + })).toStrictEqual(undefined) +}) +/* eslint-enable no-template-curly-in-string */ + test.todo('--tab-width') test('--dry-run', () => { diff --git a/tools/tests/tester/story_outputs.test.ts b/tools/tests/tester/story_outputs.test.ts new file mode 100644 index 000000000..4409936b3 --- /dev/null +++ b/tools/tests/tester/story_outputs.test.ts @@ -0,0 +1,71 @@ +/* +* 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 { ChapterOutput } from 'tester/ChapterOutput' +import { StoryOutputs } from 'tester/StoryOutputs' + +const story_outputs = new StoryOutputs({ + chapter_id: new ChapterOutput({ + x: 1, + y: 2 + }) + }) + + /* eslint-disable no-template-curly-in-string */ + test('resolve_string', () => { + expect(story_outputs.resolve_string('${chapter_id.x}')).toEqual(1) + expect(story_outputs.resolve_string('some_str')).toEqual('some_str') + }) + /* eslint-enable no-template-curly-in-string */ + + test('resolve_value', () => { + /* eslint-disable no-template-curly-in-string */ + const value = { + a: '${chapter_id.x}', + b: ['${chapter_id.x}', '${chapter_id.y}', 3], + c: { + d: '${chapter_id.x}', + e: 'str', + f: true + }, + g: 123 + } + /* eslint-enable no-template-curly-in-string */ + expect(story_outputs.resolve_value(value)).toEqual( + { + a: 1, + b: [1, 2, 3], + c: { + d: 1, + e: 'str', + f: true + }, + g: 123 + } + ) + }) + + test('resolve_params', () => { + /* eslint-disable no-template-curly-in-string */ + const parameters = { + a: '${chapter_id.x}', + b: '${chapter_id.y}', + c: 3, + d: 'str' + } + /* eslint-enable no-template-curly-in-string */ + expect(story_outputs.resolve_params(parameters)).toEqual({ + a: 1, + b: 2, + c: 3, + d: 'str' + }) + }) + +