From 6667ea71fc431fd5678875e8a5c0dc751c2bb31c Mon Sep 17 00:00:00 2001 From: dblock Date: Thu, 18 Jul 2024 16:17:56 -0400 Subject: [PATCH 1/4] Added test coverage. Signed-off-by: dblock --- CHANGELOG.md | 1 + tools/src/tester/ChapterEvaluator.ts | 1 + tools/src/tester/MergedOpenApiSpec.ts | 13 +++- tools/src/tester/ResultLogger.ts | 16 ++++- tools/src/tester/StoryEvaluator.ts | 10 +-- tools/src/tester/TestRunner.ts | 13 ++-- tools/src/tester/test.ts | 3 +- tools/src/tester/types/eval.types.ts | 9 ++- tools/tests/tester/MergedOpenApiSpec.test.ts | 57 ++++++++++------- .../fixtures/evals/error/chapter_error.yaml | 1 + .../fixtures/evals/failed/invalid_data.yaml | 5 ++ .../fixtures/evals/failed/not_found.yaml | 5 +- tools/tests/tester/fixtures/evals/passed.yaml | 8 +++ .../specs/complete/namespaces/nodes.yaml | 61 +++++++++++++++++++ tools/tests/tester/integ/TestRunner.test.ts | 6 +- 15 files changed, 166 insertions(+), 43 deletions(-) create mode 100644 tools/tests/tester/fixtures/specs/complete/namespaces/nodes.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d4e347b0..e18412102 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Added `is_hidden` to `/{index}/_alias/{name}` and `/{index}/_aliases/{name}` ([#429](https://github.com/opensearch-project/opensearch-api-specification/pull/429)) - Added `ignore_unmapped` to `GeoDistanceQuery` ([#427](https://github.com/opensearch-project/opensearch-api-specification/pull/427)) - Added missing variants of `indices.put_alias` ([#434](https://github.com/opensearch-project/opensearch-api-specification/pull/434)) +- Added test coverage ([#443](https://github.com/opensearch-project/opensearch-api-specification/pull/443)) ### Changed diff --git a/tools/src/tester/ChapterEvaluator.ts b/tools/src/tester/ChapterEvaluator.ts index 445c3e1c0..a75b2094e 100644 --- a/tools/src/tester/ChapterEvaluator.ts +++ b/tools/src/tester/ChapterEvaluator.ts @@ -48,6 +48,7 @@ export default class ChapterEvaluator { const output_values = ChapterOutput.extract_output_values(response, chapter.output) return { title: chapter.synopsis, + path: `${chapter.method} ${chapter.path}`, overall: { result: overall_result(Object.values(params).concat([ request_body, status, payload_body_evaluation, payload_schema_evaluation ]).concat(output_values ? [output_values] : [])) }, diff --git a/tools/src/tester/MergedOpenApiSpec.ts b/tools/src/tester/MergedOpenApiSpec.ts index 93a1e25dc..56ee8b7a9 100644 --- a/tools/src/tester/MergedOpenApiSpec.ts +++ b/tools/src/tester/MergedOpenApiSpec.ts @@ -9,9 +9,10 @@ import { OpenAPIV3 } from 'openapi-types' import { Logger } from '../Logger' -import { determine_possible_schema_types, SpecificationContext } from '../_utils'; +import { determine_possible_schema_types, HTTP_METHODS, SpecificationContext } from '../_utils'; import { SchemaVisitor } from '../_utils/SpecificationVisitor'; import OpenApiMerger from '../merger/OpenApiMerger'; +import _ from 'lodash'; // An augmented spec with additionalProperties: false. export default class MergedOpenApiSpec { @@ -37,6 +38,16 @@ export default class MergedOpenApiSpec { return (this.spec().info as any)['x-api-version'] } + paths(): Record { + var obj: Record = {} + _.entries(this.spec().paths).forEach(([path, ops]) => { + obj[path] = _.entries(_.pick(ops, HTTP_METHODS)).map(([verb, _]) => { + return verb + }) + }) + return obj + } + private inject_additional_properties(ctx: SpecificationContext, spec: OpenAPIV3.Document): void { const visitor = new SchemaVisitor((ctx, schema) => { // If already has unevaluatedProperties then don't set diff --git a/tools/src/tester/ResultLogger.ts b/tools/src/tester/ResultLogger.ts index 1032fc087..7446ead94 100644 --- a/tools/src/tester/ResultLogger.ts +++ b/tools/src/tester/ResultLogger.ts @@ -7,16 +7,20 @@ * compatible open source license. */ -import { type ChapterEvaluation, type Evaluation, Result, type StoryEvaluation } from './types/eval.types' +import { type StoryEvaluations, type ChapterEvaluation, type Evaluation, Result, type StoryEvaluation } from './types/eval.types' import { overall_result } from './helpers' import * as ansi from './Ansi' +import _ from 'lodash' +import MergedOpenApiSpec from './MergedOpenApiSpec' export interface ResultLogger { log: (evaluation: StoryEvaluation) => void + log_coverage: (_spec: MergedOpenApiSpec, evaluations: StoryEvaluations) => void } export class NoOpResultLogger implements ResultLogger { log (_: StoryEvaluation): void { } + log_coverage(_spec: MergedOpenApiSpec, _evaluations: StoryEvaluations): void { } } export class ConsoleResultLogger implements ResultLogger { @@ -38,6 +42,16 @@ export class ConsoleResultLogger implements ResultLogger { if (with_padding) console.log() } + log_coverage(spec: MergedOpenApiSpec, evaluations: StoryEvaluations): void { + const evaluated_paths = _.uniq(_.compact(_.flatten(_.map(evaluations.evaluations, (evaluation) => + _.map(evaluation.chapters, (chapter) => chapter.path) + )))) + + const total_paths = Object.values(spec.paths()).reduce((acc, methods) => acc + methods.length, 0); + console.log() + console.log(`Tested ${evaluated_paths.length}/${total_paths} paths.`) + } + #log_story ({ result, full_path, display_path, message }: StoryEvaluation): void { this.#log_evaluation({ result, message: message ?? full_path }, ansi.cyan(ansi.b(display_path))) } diff --git a/tools/src/tester/StoryEvaluator.ts b/tools/src/tester/StoryEvaluator.ts index db751e543..719437dc0 100644 --- a/tools/src/tester/StoryEvaluator.ts +++ b/tools/src/tester/StoryEvaluator.ts @@ -25,8 +25,8 @@ export default class StoryEvaluator { this._supplemental_chapter_evaluator = supplemental_chapter_evaluator } - async evaluate({ story, display_path, full_path }: StoryFile, version: string, dry_run: boolean = false): Promise { - if (story.version !== undefined && !semver.satisfies(version, story.version)) { + async evaluate({ story, display_path, full_path }: StoryFile, version?: string, dry_run: boolean = false): Promise { + if (version != undefined && story.version !== undefined && !semver.satisfies(version, story.version)) { return { result: Result.SKIPPED, display_path, @@ -42,7 +42,7 @@ export default class StoryEvaluator { } 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, version, dry_run, story_outputs) + const chapters = await this.#evaluate_chapters(story.chapters, prologue_errors, dry_run, story_outputs, version) const { evaluations: epilogues } = await this.#evaluate_supplemental_chapters(story.epilogues ?? [], dry_run, story_outputs) return { display_path, @@ -55,13 +55,13 @@ export default class StoryEvaluator { } } - async #evaluate_chapters(chapters: Chapter[], has_errors: boolean, version: string, dry_run: boolean, story_outputs: StoryOutputs): Promise { + async #evaluate_chapters(chapters: Chapter[], has_errors: boolean, dry_run: boolean, story_outputs: StoryOutputs, version?: string): 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 if (chapter.version !== undefined && !semver.satisfies(version, chapter.version)) { + } else if (version != undefined && chapter.version !== undefined && !semver.satisfies(version, chapter.version)) { const title = chapter.synopsis || `${chapter.method} ${chapter.path}` evaluations.push({ title, overall: { result: Result.SKIPPED, message: `Skipped because version ${version} does not satisfy ${chapter.version}.`, error: undefined } }) } else { diff --git a/tools/src/tester/TestRunner.ts b/tools/src/tester/TestRunner.ts index 72f38d4f4..8962931f4 100644 --- a/tools/src/tester/TestRunner.ts +++ b/tools/src/tester/TestRunner.ts @@ -8,11 +8,11 @@ */ import type StoryEvaluator from './StoryEvaluator' -import { type StoryFile } from './types/eval.types' +import { StoryEvaluations, 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 { Result } from './types/eval.types' import { type ResultLogger } from './ResultLogger' import { basename, resolve } from 'path' import type StoryValidator from "./StoryValidator"; @@ -32,10 +32,10 @@ export default class TestRunner { this._result_logger = result_logger } - async run (story_path: string, version: string = '2.15.0', dry_run: boolean = false): Promise<{ evaluations: StoryEvaluation[], failed: boolean }> { + async run (story_path: string, version?: string, dry_run: boolean = false): Promise<{ results: StoryEvaluations, failed: boolean }> { let failed = false const story_files = this.#sort_story_files(this.#collect_story_files(resolve(story_path), '', '')) - const evaluations: StoryEvaluation[] = [] + const results: StoryEvaluations = { evaluations: [] } if (!dry_run) { const info = await this._http_client.wait_until_available() @@ -45,11 +45,12 @@ export default class TestRunner { for (const story_file of story_files) { const evaluation = this._story_validator.validate(story_file) ?? await this._story_evaluator.evaluate(story_file, version, dry_run) - evaluations.push(evaluation) + results.evaluations.push(evaluation) this._result_logger.log(evaluation) if ([Result.ERROR, Result.FAILED].includes(evaluation.result)) failed = true } - return { evaluations, failed } + + return { results, failed } } #collect_story_files (folder: string, file: string, prefix: string): StoryFile[] { diff --git a/tools/src/tester/test.ts b/tools/src/tester/test.ts index 6fa7b5f68..a231af72f 100644 --- a/tools/src/tester/test.ts +++ b/tools/src/tester/test.ts @@ -62,7 +62,8 @@ const runner = new TestRunner(http_client, story_validator, story_evaluator, res runner.run(opts.testsPath, spec.api_version(), opts.dryRun) .then( - ({ failed }) => { + ({ results, failed }) => { + result_logger.log_coverage(spec, results) if (failed) process.exit(1) }, err => { throw err }) diff --git a/tools/src/tester/types/eval.types.ts b/tools/src/tester/types/eval.types.ts index f8fe386f9..0097361e3 100644 --- a/tools/src/tester/types/eval.types.ts +++ b/tools/src/tester/types/eval.types.ts @@ -28,9 +28,14 @@ export interface StoryEvaluation { prologues?: ChapterEvaluation[] } +export interface StoryEvaluations { + evaluations: StoryEvaluation[] +} + export interface ChapterEvaluation { - title: string - overall: Evaluation + title: string, + overall: Evaluation, + path?: string, request?: { parameters?: Record request_body?: Evaluation diff --git a/tools/tests/tester/MergedOpenApiSpec.test.ts b/tools/tests/tester/MergedOpenApiSpec.test.ts index 2acda940d..2c656082d 100644 --- a/tools/tests/tester/MergedOpenApiSpec.test.ts +++ b/tools/tests/tester/MergedOpenApiSpec.test.ts @@ -10,36 +10,47 @@ import { Logger } from 'Logger' import MergedOpenApiSpec from "tester/MergedOpenApiSpec" -describe('unevaluatedProperties', () => { +describe('merged API spec', () => { const spec = new MergedOpenApiSpec('tools/tests/tester/fixtures/specs/complete', new Logger()) - const responses: any = spec.spec().components?.responses test('has an api version', () => { expect(spec.api_version()).toEqual('1.2.3') }) - test('is added with required fields', () => { - const schema = responses['info@200'].content['application/json'].schema - expect(schema.unevaluatedProperties).toEqual({ not: true, errorMessage: 'property is not defined in the spec' }) + test('paths', () => { + expect(spec.paths()).toEqual({ + '/_nodes/{id}': ['get', 'post'], + '/index': ['get'], + '/nodes': ['get'] + }) }) - test('is added when no required fields', () => { - const schema = responses['info@500'].content['application/json'].schema - expect(schema.unevaluatedProperties).toEqual({ not: true, errorMessage: 'property is not defined in the spec' }) - }) - - test('is not added to empty object schema', () => { - const schema = responses['info@503'].content['application/json'].schema - expect(schema.unevaluatedProperties).toBeUndefined() - }) - - test('is not added when true', () => { - const schema = responses['info@201'].content['application/json'].schema - expect(schema.unevaluatedProperties).toEqual(true) - }) - - test('is not added when object', () => { - const schema = responses['info@404'].content['application/json'].schema - expect(schema.unevaluatedProperties).toEqual({ type: 'object' }) + describe('unevaluatedProperties', () => { + const responses: any = spec.spec().components?.responses + + test('is added with required fields', () => { + const schema = responses['info@200'].content['application/json'].schema + expect(schema.unevaluatedProperties).toEqual({ not: true, errorMessage: 'property is not defined in the spec' }) + }) + + test('is added when no required fields', () => { + const schema = responses['info@500'].content['application/json'].schema + expect(schema.unevaluatedProperties).toEqual({ not: true, errorMessage: 'property is not defined in the spec' }) + }) + + test('is not added to empty object schema', () => { + const schema = responses['info@503'].content['application/json'].schema + expect(schema.unevaluatedProperties).toBeUndefined() + }) + + test('is not added when true', () => { + const schema = responses['info@201'].content['application/json'].schema + expect(schema.unevaluatedProperties).toEqual(true) + }) + + test('is not added when object', () => { + const schema = responses['info@404'].content['application/json'].schema + expect(schema.unevaluatedProperties).toEqual({ type: 'object' }) + }) }) }) diff --git a/tools/tests/tester/fixtures/evals/error/chapter_error.yaml b/tools/tests/tester/fixtures/evals/error/chapter_error.yaml index 704c71ba4..37f1baba4 100644 --- a/tools/tests/tester/fixtures/evals/error/chapter_error.yaml +++ b/tools/tests/tester/fixtures/evals/error/chapter_error.yaml @@ -17,6 +17,7 @@ chapters: - title: This chapter show throw an error. overall: result: ERROR + path: DELETE /{index} request: parameters: {} request_body: diff --git a/tools/tests/tester/fixtures/evals/failed/invalid_data.yaml b/tools/tests/tester/fixtures/evals/failed/invalid_data.yaml index ec0c9043f..a05ae7aa7 100644 --- a/tools/tests/tester/fixtures/evals/failed/invalid_data.yaml +++ b/tools/tests/tester/fixtures/evals/failed/invalid_data.yaml @@ -10,6 +10,7 @@ chapters: - title: This chapter should fail because the parameter is invalid. overall: result: FAILED + path: PUT /{index} request: parameters: index: @@ -27,6 +28,7 @@ chapters: - title: This chapter should fail because the request body is invalid. overall: result: FAILED + path: PUT /{index} request: parameters: index: @@ -44,6 +46,7 @@ chapters: - title: This chapter should fail because the response content type does not match. overall: result: FAILED + path: GET /_cat/indices/{index} request: parameters: format: @@ -63,6 +66,7 @@ chapters: - title: This chapter should fail because the response data and schema are invalid. overall: result: FAILED + path: DELETE /{index} request: parameters: index: @@ -81,6 +85,7 @@ chapters: - title: This chapter should fail because the response status does not match. overall: result: ERROR + path: PUT /{index} request: parameters: index: diff --git a/tools/tests/tester/fixtures/evals/failed/not_found.yaml b/tools/tests/tester/fixtures/evals/failed/not_found.yaml index 766fb3b99..72bc6318b 100644 --- a/tools/tests/tester/fixtures/evals/failed/not_found.yaml +++ b/tools/tests/tester/fixtures/evals/failed/not_found.yaml @@ -14,6 +14,7 @@ chapters: - title: This chapter should fail because the parameter is not defined in the spec. overall: result: FAILED + path: PUT /{index} request: parameters: index: @@ -33,6 +34,7 @@ chapters: - title: This chapter should fail because the request body is not defined in the spec. overall: result: FAILED + path: HEAD /{index} request: parameters: index: @@ -50,6 +52,7 @@ chapters: - title: This chapter should fail because the response is not defined in the spec. overall: result: FAILED + path: DELETE /{index} request: parameters: index: @@ -68,4 +71,4 @@ chapters: epilogues: - title: DELETE /books overall: - result: PASSED \ No newline at end of file + result: PASSED diff --git a/tools/tests/tester/fixtures/evals/passed.yaml b/tools/tests/tester/fixtures/evals/passed.yaml index 6808a9da1..375b24f26 100644 --- a/tools/tests/tester/fixtures/evals/passed.yaml +++ b/tools/tests/tester/fixtures/evals/passed.yaml @@ -10,6 +10,7 @@ chapters: - title: This PUT /{index} chapter should pass. overall: result: PASSED + path: PUT /{index} request: parameters: index: @@ -26,6 +27,7 @@ chapters: - title: This GET /_cat chapter returns text/plain and should pass. overall: result: PASSED + path: GET /_cat request: parameters: {} request_body: @@ -40,6 +42,7 @@ chapters: - title: This GET /_cat/health chapter returns application/json and should pass. overall: result: PASSED + path: GET /_cat/health request: parameters: format: @@ -56,6 +59,7 @@ chapters: - title: This GET /_cat/health chapter returns application/yaml and should pass. overall: result: PASSED + path: GET /_cat/health request: parameters: format: @@ -72,6 +76,7 @@ chapters: - title: This GET /_cat/health chapter returns application/cbor and should pass. overall: result: PASSED + path: GET /_cat/health request: parameters: format: @@ -88,6 +93,7 @@ chapters: - title: This GET /_cat/health chapter returns application/smile and should pass. overall: result: PASSED + path: GET /_cat/health request: parameters: format: @@ -104,6 +110,7 @@ chapters: - title: This GET /_cat/health should run (default). overall: result: PASSED + path: GET /_cat/health request: parameters: format: @@ -120,6 +127,7 @@ chapters: - title: This GET /_cat/health should run (~> 2.x). overall: result: PASSED + path: GET /_cat/health request: parameters: format: diff --git a/tools/tests/tester/fixtures/specs/complete/namespaces/nodes.yaml b/tools/tests/tester/fixtures/specs/complete/namespaces/nodes.yaml new file mode 100644 index 000000000..87abb9dbe --- /dev/null +++ b/tools/tests/tester/fixtures/specs/complete/namespaces/nodes.yaml @@ -0,0 +1,61 @@ +openapi: 3.1.0 +info: + title: OpenSearch API + description: OpenSearch API + version: 1.0.0 +paths: + /nodes: + get: + operationId: nodes.0 + responses: + '200': + $ref: '#/components/responses/nodes.info@200' + /_nodes/{id}: + get: + operationId: nodes.info.1 + x-operation-group: nodes.info + x-version-added: '1.0' + description: Returns information about nodes in the cluster. + parameters: + - $ref: '#/components/parameters/nodes.info::path.id' + responses: + '200': + $ref: '#/components/responses/nodes.info@200' + post: + operationId: nodes.info.1 + x-operation-group: nodes.info + x-version-added: '1.0' + description: Returns information about nodes in the cluster. + parameters: + - $ref: '#/components/parameters/nodes.info::path.id' + requestBody: + $ref: '#/components/requestBodies/nodes.info' + responses: + '200': + $ref: '#/components/responses/nodes.info@200' +components: + requestBodies: + nodes.info: + content: + application/json: + schema: + type: object + properties: + _all: + type: boolean + description: Nodes options. + parameters: + nodes.info::path.id: + in: path + name: id + description: Node ID. + required: true + schema: + type: string + responses: + nodes.info@200: + description: All nodes. + content: + application/json: + schema: + type: object diff --git a/tools/tests/tester/integ/TestRunner.test.ts b/tools/tests/tester/integ/TestRunner.test.ts index 8a706b6b1..4e06a109c 100644 --- a/tools/tests/tester/integ/TestRunner.test.ts +++ b/tools/tests/tester/integ/TestRunner.test.ts @@ -16,13 +16,13 @@ test('stories folder', async () => { const info = await opensearch_http_client.wait_until_available() expect(info.version).toBeDefined() - const result = await test_runner.run('tools/tests/tester/fixtures/stories') + const run = await test_runner.run('tools/tests/tester/fixtures/stories') - expect(result.failed).toBeTruthy() + expect(run.failed).toBeTruthy() const actual_evaluations: Array> = [] - for (const evaluation of result.evaluations) { + for (const evaluation of run.results.evaluations) { const { full_path, ...rest } = flatten_errors(evaluation) expect(full_path.endsWith(rest.display_path)).toBeTruthy() actual_evaluations.push(rest) From c502cd386d47aab306c77afc6c1200614b670592 Mon Sep 17 00:00:00 2001 From: dblock Date: Fri, 19 Jul 2024 13:17:26 -0400 Subject: [PATCH 2/4] Extract TestResults. Signed-off-by: dblock --- tools/src/tester/ResultLogger.ts | 18 ++++------- tools/src/tester/TestResults.ts | 32 ++++++++++++++++++ tools/src/tester/test.ts | 5 ++- tools/tests/tester/ResultLogger.test.ts | 26 +++++++++++++++ tools/tests/tester/TestResults.test.ts | 43 +++++++++++++++++++++++++ 5 files changed, 111 insertions(+), 13 deletions(-) create mode 100644 tools/src/tester/TestResults.ts create mode 100644 tools/tests/tester/TestResults.test.ts diff --git a/tools/src/tester/ResultLogger.ts b/tools/src/tester/ResultLogger.ts index 7446ead94..aa1518499 100644 --- a/tools/src/tester/ResultLogger.ts +++ b/tools/src/tester/ResultLogger.ts @@ -7,20 +7,19 @@ * compatible open source license. */ -import { type StoryEvaluations, type ChapterEvaluation, type Evaluation, Result, type StoryEvaluation } from './types/eval.types' +import { type ChapterEvaluation, type Evaluation, Result, type StoryEvaluation } from './types/eval.types' import { overall_result } from './helpers' import * as ansi from './Ansi' -import _ from 'lodash' -import MergedOpenApiSpec from './MergedOpenApiSpec' +import TestResults from './TestResults' export interface ResultLogger { log: (evaluation: StoryEvaluation) => void - log_coverage: (_spec: MergedOpenApiSpec, evaluations: StoryEvaluations) => void + log_coverage: (_results: TestResults) => void } export class NoOpResultLogger implements ResultLogger { log (_: StoryEvaluation): void { } - log_coverage(_spec: MergedOpenApiSpec, _evaluations: StoryEvaluations): void { } + log_coverage(_results: TestResults): void { } } export class ConsoleResultLogger implements ResultLogger { @@ -42,14 +41,9 @@ export class ConsoleResultLogger implements ResultLogger { if (with_padding) console.log() } - log_coverage(spec: MergedOpenApiSpec, evaluations: StoryEvaluations): void { - const evaluated_paths = _.uniq(_.compact(_.flatten(_.map(evaluations.evaluations, (evaluation) => - _.map(evaluation.chapters, (chapter) => chapter.path) - )))) - - const total_paths = Object.values(spec.paths()).reduce((acc, methods) => acc + methods.length, 0); + log_coverage(results: TestResults): void { console.log() - console.log(`Tested ${evaluated_paths.length}/${total_paths} paths.`) + console.log(`Tested ${results.evaluated_paths_count()}/${results.spec_paths_count()} paths.`) } #log_story ({ result, full_path, display_path, message }: StoryEvaluation): void { diff --git a/tools/src/tester/TestResults.ts b/tools/src/tester/TestResults.ts new file mode 100644 index 000000000..b7c3291f8 --- /dev/null +++ b/tools/src/tester/TestResults.ts @@ -0,0 +1,32 @@ +/* +* 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 _ from "lodash"; +import MergedOpenApiSpec from "./MergedOpenApiSpec"; +import { StoryEvaluations } from "./types/eval.types"; + +export default class TestResults { + protected _spec: MergedOpenApiSpec + protected _evaluations: StoryEvaluations + + constructor(spec: MergedOpenApiSpec, evaluations: StoryEvaluations) { + this._spec = spec + this._evaluations = evaluations + } + + evaluated_paths_count(): number { + return _.uniq(_.compact(_.flatten(_.map(this._evaluations.evaluations, (evaluation) => + _.map(evaluation.chapters, (chapter) => chapter.path) + )))).length + } + + spec_paths_count(): number { + return Object.values(this._spec.paths()).reduce((acc, methods) => acc + methods.length, 0); + } +} diff --git a/tools/src/tester/test.ts b/tools/src/tester/test.ts index a231af72f..8ed569060 100644 --- a/tools/src/tester/test.ts +++ b/tools/src/tester/test.ts @@ -28,6 +28,7 @@ import * as process from 'node:process' import SupplementalChapterEvaluator from './SupplementalChapterEvaluator' import MergedOpenApiSpec from './MergedOpenApiSpec' import StoryValidator from "./StoryValidator"; +import TestResults from './TestResults' const command = new Command() .description('Run test stories against the OpenSearch spec.') @@ -44,6 +45,7 @@ const command = new Command() .addOption(OPENSEARCH_USERNAME_OPTION) .addOption(OPENSEARCH_PASSWORD_OPTION) .addOption(OPENSEARCH_INSECURE_OPTION) + .addOption(new Option('--coverage ', 'path to write test coverage results to')) .allowExcessArguments(false) .parse() @@ -63,7 +65,8 @@ const runner = new TestRunner(http_client, story_validator, story_evaluator, res runner.run(opts.testsPath, spec.api_version(), opts.dryRun) .then( ({ results, failed }) => { - result_logger.log_coverage(spec, results) + const test_results = new TestResults(spec, results) + result_logger.log_coverage(test_results) if (failed) process.exit(1) }, err => { throw err }) diff --git a/tools/tests/tester/ResultLogger.test.ts b/tools/tests/tester/ResultLogger.test.ts index 8c0470dae..2eea82b2d 100644 --- a/tools/tests/tester/ResultLogger.test.ts +++ b/tools/tests/tester/ResultLogger.test.ts @@ -10,6 +10,8 @@ import { ConsoleResultLogger } from "tester/ResultLogger" import { Result } from "tester/types/eval.types" import * as ansi from 'tester/Ansi' +import MergedOpenApiSpec from "tester/MergedOpenApiSpec" +import TestResults from "tester/TestResults" describe('ConsoleResultLogger', () => { let log: jest.Mock @@ -52,6 +54,30 @@ describe('ConsoleResultLogger', () => { [] ]) }) + + test('log_coverage', () => { + const spec = new MergedOpenApiSpec('tools/tests/tester/fixtures/specs/complete') + const test_results = new TestResults(spec, { evaluations: [{ + result: Result.PASSED, + display_path: 'path', + full_path: 'path', + description: 'description', + chapters: [ + { + title: 'title', + overall: { result: Result.PASSED }, + path: 'path' + } + ] + }] }) + + logger.log_coverage(test_results) + + expect(log.mock.calls).toEqual([ + [], + ['Tested 1/4 paths.'] + ]) + }) }) describe('verbose=false', () => { diff --git a/tools/tests/tester/TestResults.test.ts b/tools/tests/tester/TestResults.test.ts new file mode 100644 index 000000000..d74781038 --- /dev/null +++ b/tools/tests/tester/TestResults.test.ts @@ -0,0 +1,43 @@ +/* +* 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 MergedOpenApiSpec from "tester/MergedOpenApiSpec" +import TestResults from "tester/TestResults" +import { Result } from "tester/types/eval.types" + +describe('TestResults', () => { + const spec = new MergedOpenApiSpec('tools/tests/tester/fixtures/specs/complete') + + const evaluations = [{ + result: Result.PASSED, + display_path: 'path', + full_path: 'full_path', + description: 'description', + message: 'message', + chapters: [{ + title: 'title', + overall: { + result: Result.PASSED + }, + path: 'path' + }], + epilogues: [], + prologues: [] + }] + + const test_results = new TestResults(spec, { evaluations }) + + test('evaluated_paths_count', () => { + expect(test_results.evaluated_paths_count()).toEqual(1) + }) + + test('spec_paths_count', () => { + expect(test_results.spec_paths_count()).toEqual(4) + }) +}) From 1236f77f4607bb88b783231a5ee4e66714b07c41 Mon Sep 17 00:00:00 2001 From: dblock Date: Fri, 19 Jul 2024 13:33:02 -0400 Subject: [PATCH 3/4] Write coverage data to JSON. Signed-off-by: dblock --- .github/workflows/test-spec.yml | 8 +++++++- tools/src/tester/TestResults.ts | 13 +++++++++++++ tools/src/tester/test.ts | 6 ++++++ tools/src/tester/types/test.types.ts | 13 +++++++++++++ tools/tests/tester/TestResults.test.ts | 11 +++++++++++ 5 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 tools/src/tester/types/test.types.ts diff --git a/.github/workflows/test-spec.yml b/.github/workflows/test-spec.yml index 8e290cdf2..6686b8d9e 100644 --- a/.github/workflows/test-spec.yml +++ b/.github/workflows/test-spec.yml @@ -54,4 +54,10 @@ jobs: run: docker-compose up -d - name: Run Tests - run: npm run test:spec -- --opensearch-insecure \ No newline at end of file + run: npm run test:spec -- --opensearch-insecure --coverage test-spec-coverage-${{ matrix.entry.version }}.json + + - name: Upload Test Coverage Results + uses: actions/upload-artifact@v4 + with: + name: test-spec-coverage-${{ matrix.entry.version }} + path: test-spec-coverage-${{ matrix.entry.version }}.json \ No newline at end of file diff --git a/tools/src/tester/TestResults.ts b/tools/src/tester/TestResults.ts index b7c3291f8..112a5bbf3 100644 --- a/tools/src/tester/TestResults.ts +++ b/tools/src/tester/TestResults.ts @@ -10,6 +10,8 @@ import _ from "lodash"; import MergedOpenApiSpec from "./MergedOpenApiSpec"; import { StoryEvaluations } from "./types/eval.types"; +import { SpecTestCoverage } from "./types/test.types"; +import { write_json } from "../helpers"; export default class TestResults { protected _spec: MergedOpenApiSpec @@ -29,4 +31,15 @@ export default class TestResults { spec_paths_count(): number { return Object.values(this._spec.paths()).reduce((acc, methods) => acc + methods.length, 0); } + + test_coverage(): SpecTestCoverage { + return { + evaluated_paths_count: this.evaluated_paths_count(), + paths_count: this.spec_paths_count() + } + } + + write_coverage(file_path: string): void { + write_json(file_path, this.test_coverage()) + } } diff --git a/tools/src/tester/test.ts b/tools/src/tester/test.ts index 8ed569060..5c55a5446 100644 --- a/tools/src/tester/test.ts +++ b/tools/src/tester/test.ts @@ -65,8 +65,14 @@ const runner = new TestRunner(http_client, story_validator, story_evaluator, res runner.run(opts.testsPath, spec.api_version(), opts.dryRun) .then( ({ results, failed }) => { + const test_results = new TestResults(spec, results) result_logger.log_coverage(test_results) + if (opts.coverage !== undefined) { + console.log(`Writing ${opts.coverage} ...`) + test_results.write_coverage(opts.coverage) + } + if (failed) process.exit(1) }, err => { throw err }) diff --git a/tools/src/tester/types/test.types.ts b/tools/src/tester/types/test.types.ts new file mode 100644 index 000000000..535770099 --- /dev/null +++ b/tools/src/tester/types/test.types.ts @@ -0,0 +1,13 @@ +/* +* 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. +*/ + +export interface SpecTestCoverage { + paths_count: number + evaluated_paths_count: number +} diff --git a/tools/tests/tester/TestResults.test.ts b/tools/tests/tester/TestResults.test.ts index d74781038..d6a5b239d 100644 --- a/tools/tests/tester/TestResults.test.ts +++ b/tools/tests/tester/TestResults.test.ts @@ -10,6 +10,7 @@ import MergedOpenApiSpec from "tester/MergedOpenApiSpec" import TestResults from "tester/TestResults" import { Result } from "tester/types/eval.types" +import fs from 'fs' describe('TestResults', () => { const spec = new MergedOpenApiSpec('tools/tests/tester/fixtures/specs/complete') @@ -40,4 +41,14 @@ describe('TestResults', () => { test('spec_paths_count', () => { expect(test_results.spec_paths_count()).toEqual(4) }) + + test('write_coverage', () => { + const filename = 'coverage.json' + test_results.write_coverage(filename) + expect(JSON.parse(fs.readFileSync(filename, 'utf8'))).toEqual({ + evaluated_paths_count: 1, + paths_count: 4 + }) + fs.unlinkSync(filename) + }) }) From f13fa2caf87947a80152496f1b257f8e1d2153a4 Mon Sep 17 00:00:00 2001 From: dblock Date: Fri, 19 Jul 2024 14:00:23 -0400 Subject: [PATCH 4/4] Report test coverage in a PR comment. Signed-off-by: dblock --- .cspell | 1 + .../pr-test-coverage-analysis.template.md | 8 +++ .github/workflows/pr-comment.yml | 3 +- .github/workflows/test-spec.yml | 57 ++++++++++++++++++- tools/src/tester/TestResults.ts | 6 +- tools/src/tester/types/test.types.ts | 6 +- tools/tests/tester/TestResults.test.ts | 6 +- 7 files changed, 77 insertions(+), 10 deletions(-) create mode 100644 .github/pr-comment-templates/pr-test-coverage-analysis.template.md diff --git a/.cspell b/.cspell index f292a14d7..37a004c69 100644 --- a/.cspell +++ b/.cspell @@ -47,6 +47,7 @@ aarch actiongroup actiongroups aggregatable +argjson asciifolding authc authinfo diff --git a/.github/pr-comment-templates/pr-test-coverage-analysis.template.md b/.github/pr-comment-templates/pr-test-coverage-analysis.template.md new file mode 100644 index 000000000..2dcd0ae02 --- /dev/null +++ b/.github/pr-comment-templates/pr-test-coverage-analysis.template.md @@ -0,0 +1,8 @@ +## Spec Test Coverage Analysis +{{with .test_coverage}} + +| Total | Tested | +|-------------------|-----------------------------| +| {{.paths_count}} | {{.evaluated_paths_count}} | + +{{end}} \ No newline at end of file diff --git a/.github/workflows/pr-comment.yml b/.github/workflows/pr-comment.yml index f66217f69..037cca862 100644 --- a/.github/workflows/pr-comment.yml +++ b/.github/workflows/pr-comment.yml @@ -4,6 +4,7 @@ on: workflow_run: workflows: - Analyze PR Changes + - Test Spec types: - completed @@ -71,4 +72,4 @@ jobs: comment-id: ${{ steps.fc.outputs.comment-id }} issue-number: ${{ env.PR_NUMBER }} body: ${{ steps.render.outputs.result }} - edit-mode: replace \ No newline at end of file + edit-mode: replace diff --git a/.github/workflows/test-spec.yml b/.github/workflows/test-spec.yml index 6686b8d9e..6395778a1 100644 --- a/.github/workflows/test-spec.yml +++ b/.github/workflows/test-spec.yml @@ -54,10 +54,61 @@ jobs: run: docker-compose up -d - name: Run Tests - run: npm run test:spec -- --opensearch-insecure --coverage test-spec-coverage-${{ matrix.entry.version }}.json + run: | + npm run test:spec -- --opensearch-insecure --coverage coverage/test-spec-coverage-${{ matrix.entry.version }}.json - name: Upload Test Coverage Results uses: actions/upload-artifact@v4 with: - name: test-spec-coverage-${{ matrix.entry.version }} - path: test-spec-coverage-${{ matrix.entry.version }}.json \ No newline at end of file + name: coverage-${{ matrix.entry.version }} + path: coverage/test-spec-coverage-${{ matrix.entry.version }}.json + + merge-coverage: + runs-on: ubuntu-latest + if: github.event_name == 'pull_request' + needs: test-opensearch-spec + steps: + - uses: actions/checkout@v3 + + - name: Download Spec Coverage Data + uses: actions/download-artifact@v4 + with: + path: coverage + + - name: Combine Test Coverage Data + shell: bash -eo pipefail {0} + run: | + jq -sc ' + map(to_entries) | + flatten | + group_by(.key) | + map({key: .[0].key, value: map(.value) | max}) | + from_entries | + .' $(find ./coverage -name "test-spec-coverage-*.json") > ./coverage/coverage.json + + cat ./coverage/coverage.json + + - name: Construct Comment Data Payload + shell: bash -eo pipefail {0} + run: | + jq \ + --arg pr_number ${PR_NUMBER} \ + --slurpfile test_coverage ./coverage/coverage.json \ + --null-input ' + { + "pr_number": ($pr_number), + "comment_identifier": "# Test Coverage Analysis", + "template_name": "pr-test-coverage-analysis", + "template_data": { + "test_coverage": ($test_coverage[0]) + } + } + ' | tee pr-comment.json + env: + PR_NUMBER: ${{ github.event.pull_request.number }} + + - name: Upload PR Comment Payload + uses: actions/upload-artifact@v4 + with: + name: pr-comment + path: pr-comment.json \ No newline at end of file diff --git a/tools/src/tester/TestResults.ts b/tools/src/tester/TestResults.ts index 112a5bbf3..8740d8771 100644 --- a/tools/src/tester/TestResults.ts +++ b/tools/src/tester/TestResults.ts @@ -34,8 +34,10 @@ export default class TestResults { test_coverage(): SpecTestCoverage { return { - evaluated_paths_count: this.evaluated_paths_count(), - paths_count: this.spec_paths_count() + test_coverage: { + evaluated_paths_count: this.evaluated_paths_count(), + paths_count: this.spec_paths_count() + } } } diff --git a/tools/src/tester/types/test.types.ts b/tools/src/tester/types/test.types.ts index 535770099..9ce87f311 100644 --- a/tools/src/tester/types/test.types.ts +++ b/tools/src/tester/types/test.types.ts @@ -8,6 +8,8 @@ */ export interface SpecTestCoverage { - paths_count: number - evaluated_paths_count: number + test_coverage: { + paths_count: number + evaluated_paths_count: number + } } diff --git a/tools/tests/tester/TestResults.test.ts b/tools/tests/tester/TestResults.test.ts index d6a5b239d..21eed3c62 100644 --- a/tools/tests/tester/TestResults.test.ts +++ b/tools/tests/tester/TestResults.test.ts @@ -46,8 +46,10 @@ describe('TestResults', () => { const filename = 'coverage.json' test_results.write_coverage(filename) expect(JSON.parse(fs.readFileSync(filename, 'utf8'))).toEqual({ - evaluated_paths_count: 1, - paths_count: 4 + test_coverage: { + evaluated_paths_count: 1, + paths_count: 4 + } }) fs.unlinkSync(filename) })