From 18945ccdbead2425d8917ac4440a02d44e6abd19 Mon Sep 17 00:00:00 2001 From: dblock Date: Thu, 15 Aug 2024 08:46:41 -0400 Subject: [PATCH] Added detailed test coverage report. Signed-off-by: dblock --- TESTING_GUIDE.md | 32 ++++++++++++++++++++++ tools/src/tester/ResultLogger.ts | 26 +++++++++++++++++- tools/src/tester/TestResults.ts | 34 +++++++++++++++++------- tools/src/tester/test.ts | 2 ++ tools/tests/tester/ResultLogger.test.ts | 35 +++++++++++++++++++++++++ tools/tests/tester/TestResults.test.ts | 30 ++++++++++++++++----- 6 files changed, 143 insertions(+), 16 deletions(-) diff --git a/TESTING_GUIDE.md b/TESTING_GUIDE.md index bda054ab7..2dd25fd3a 100644 --- a/TESTING_GUIDE.md +++ b/TESTING_GUIDE.md @@ -16,6 +16,9 @@ - [Warnings](#warnings) - [multiple-paths-detected](#multiple-paths-detected) - [Suppressing Warnings](#suppressing-warnings) + - [Collecting Test Coverage](#collecting-test-coverage) + - [Coverage Summary](#coverage-summary) + - [Coverage Report](#coverage-report) # Spec Testing Guide @@ -318,3 +321,32 @@ The test runner may generate warnings that can be suppressed with `warnings:`. F parameters: index: movies ``` + +## Collecting Test Coverage + +### Coverage Summary + +The test tool can generate a test coverage summary using `--coverage ` with the number of evaluated verb + path combinations, a total number of paths and the % of paths tested. + +```json +{ + "evaluated_paths_count": 214, + "paths_count": 550, + "evaluated_paths_pct": 38.91 +} +``` + +The report is then used by the [test-spec.yml](.github/workflows/test-spec.yml) workflow, uploaded with every run, combined across various versions of OpenSearch, and reported as a comment on each pull request. + +### Coverage Report + +The test tool can display detailed and hierarchal test coverage with `--coverage-report`. This is useful to identify untested paths. For example, the [put_alias.yaml](tests/default/indices/aliases/put_alias.yaml) test exercises `PUT /_alias/{name}`, but not the other verbs. The report produces the following output with the missing ones. + +``` +/_alias (4) + GET /_alias + /{name} (3) + GET /_alias/{name} + POST /_alias/{name} + HEAD /_alias/{name} +``` \ No newline at end of file diff --git a/tools/src/tester/ResultLogger.ts b/tools/src/tester/ResultLogger.ts index d2ef339c2..45cd601c6 100644 --- a/tools/src/tester/ResultLogger.ts +++ b/tools/src/tester/ResultLogger.ts @@ -11,6 +11,7 @@ import { type ChapterEvaluation, type Evaluation, Result, type StoryEvaluation } import { overall_result } from './helpers' import * as ansi from './Ansi' import TestResults from './TestResults' +import _ from 'lodash' export interface ResultLogger { log: (evaluation: StoryEvaluation) => void @@ -43,7 +44,30 @@ export class ConsoleResultLogger implements ResultLogger { log_coverage(results: TestResults): void { console.log() - console.log(`Tested ${results.evaluated_paths_count()}/${results.spec_paths_count()} paths.`) + console.log(`Tested ${results.evaluated_paths().length}/${results.spec_paths().length} paths.`) + } + + log_coverage_report(results: TestResults): void { + console.log() + console.log(`${results.unevaluated_paths().length} paths remaining.`) + const groups = _.groupBy(results.unevaluated_paths(), (path) => path.split(' ', 2)[1].split('/')[1]) + Object.entries(groups).forEach(([root, paths]) => { + this.#log_coverage_group(root, paths) + }); + } + + #log_coverage_group(key: string, paths: string[], index: number = 2): void { + if (paths.length == 0 || key == undefined) return + console.log(`${' '.repeat(index)}/${key} (${paths.length})`) + const current_level_paths = paths.filter((path) => path.split('/').length == index) + current_level_paths.forEach((path) => { + console.log(`${' '.repeat(index + 2)}${path}`) + }) + const next_level_paths = paths.filter((path) => path.split('/').length > index) + const subgroups = _.groupBy(next_level_paths, (path) => path.split('/')[index]) + Object.entries(subgroups).forEach(([root, paths]) => { + this.#log_coverage_group(root, paths, index + 1) + }); } #log_story ({ result, full_path, display_path, message, warnings }: StoryEvaluation): void { diff --git a/tools/src/tester/TestResults.ts b/tools/src/tester/TestResults.ts index c4ae26b78..676411d20 100644 --- a/tools/src/tester/TestResults.ts +++ b/tools/src/tester/TestResults.ts @@ -16,28 +16,44 @@ import { write_json } from "../helpers"; export default class TestResults { protected _spec: MergedOpenApiSpec protected _evaluations: StoryEvaluations + protected _evaluated_paths?: string[] + protected _unevaluated_paths?: string[] + protected _spec_paths?: string[] constructor(spec: MergedOpenApiSpec, evaluations: StoryEvaluations) { this._spec = spec this._evaluations = evaluations } - evaluated_paths_count(): number { - return _.uniq(_.compact(_.flatten(_.map(this._evaluations.evaluations, (evaluation) => + evaluated_paths(): string[] { + if (this._evaluated_paths !== undefined) return this._evaluated_paths + this._evaluated_paths = _.uniq(_.compact(_.flatten(_.map(this._evaluations.evaluations, (evaluation) => _.map(evaluation.chapters, (chapter) => chapter.path) - )))).length + )))) + return this._evaluated_paths } - spec_paths_count(): number { - return Object.values(this._spec.paths()).reduce((acc, methods) => acc + methods.length, 0); + unevaluated_paths(): string[] { + if (this._unevaluated_paths !== undefined) return this._unevaluated_paths + this._unevaluated_paths = this.spec_paths().filter((path => !this.evaluated_paths().includes(path))) + return this._unevaluated_paths + } + + spec_paths(): string[] { + if (this._spec_paths !== undefined) return this._spec_paths + this._spec_paths = _.uniq(Object.entries(this._spec.paths()).flatMap(([path, path_item]) => { + return Object.values(path_item).map((method) => `${method.toUpperCase()} ${path}`) + })) + + return this._spec_paths } test_coverage(): SpecTestCoverage { return { - evaluated_paths_count: this.evaluated_paths_count(), - paths_count: this.spec_paths_count(), - evaluated_paths_pct: this.spec_paths_count() > 0 ? Math.round( - this.evaluated_paths_count() / this.spec_paths_count() * 100 * 100 + evaluated_paths_count: this.evaluated_paths().length, + paths_count: this.spec_paths().length, + evaluated_paths_pct: this.spec_paths().length > 0 ? Math.round( + this.evaluated_paths().length / this.spec_paths().length * 100 * 100 ) / 100 : 0, } } diff --git a/tools/src/tester/test.ts b/tools/src/tester/test.ts index da025a34b..e781e343c 100644 --- a/tools/src/tester/test.ts +++ b/tools/src/tester/test.ts @@ -60,6 +60,7 @@ const command = new Command() .addOption(AWS_REGION_OPTION) .addOption(AWS_SERVICE_OPTION) .addOption(new Option('--coverage ', 'path to write test coverage results to')) + .addOption(new Option('--coverage-report', 'display a detailed test coverage report')) .allowExcessArguments(false) .parse() @@ -82,6 +83,7 @@ runner.run(opts.testsPath, spec.api_version(), opts.opensearchDistribution, opts const test_results = new TestResults(spec, results) result_logger.log_coverage(test_results) + if (opts.coverageReport) result_logger.log_coverage_report(test_results) if (opts.coverage !== undefined) { console.log(`Writing ${opts.coverage} ...`) test_results.write_coverage(opts.coverage) diff --git a/tools/tests/tester/ResultLogger.test.ts b/tools/tests/tester/ResultLogger.test.ts index f21e45af1..1f7e4eb53 100644 --- a/tools/tests/tester/ResultLogger.test.ts +++ b/tools/tests/tester/ResultLogger.test.ts @@ -79,6 +79,41 @@ describe('ConsoleResultLogger', () => { ]) }) + test('log_coverage_report', () => { + 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: 'GET /_nodes/{id}' + } + ] + }] }) + + logger.log_coverage_report(test_results) + + expect(log.mock.calls).not.toContain(["GET /_nodes/{id}"]) + expect(log.mock.calls).toEqual([ + [], + ["5 paths remaining."], + [" /_nodes (1)"], + [" /{id} (1)"], + [" POST /_nodes/{id}"], + [" /cluster_manager (2)"], + [" GET /cluster_manager"], + [" POST /cluster_manager"], + [" /index (1)"], + [" GET /index"], + [" /nodes (1)"], + [" GET /nodes"] + ]) + }) + test('with retries', () => { logger.log({ result: Result.PASSED, diff --git a/tools/tests/tester/TestResults.test.ts b/tools/tests/tester/TestResults.test.ts index ac06a6350..cf65cd1a0 100644 --- a/tools/tests/tester/TestResults.test.ts +++ b/tools/tests/tester/TestResults.test.ts @@ -17,7 +17,7 @@ describe('TestResults', () => { const evaluations = [{ result: Result.PASSED, - display_path: 'path', + display_path: 'PUT /{index}', full_path: 'full_path', description: 'description', message: 'message', @@ -26,7 +26,7 @@ describe('TestResults', () => { overall: { result: Result.PASSED }, - path: 'path' + path: 'PUT /{index}' }], epilogues: [], prologues: [] @@ -34,12 +34,30 @@ describe('TestResults', () => { const test_results = new TestResults(spec, { evaluations }) - test('evaluated_paths_count', () => { - expect(test_results.evaluated_paths_count()).toEqual(1) + test('unevaluated_paths', () => { + expect(test_results.unevaluated_paths()).toEqual([ + "GET /_nodes/{id}", + "POST /_nodes/{id}", + "GET /cluster_manager", + "POST /cluster_manager", + "GET /index", + "GET /nodes" + ]) }) - test('spec_paths_count', () => { - expect(test_results.spec_paths_count()).toEqual(6) + test('evaluated_paths', () => { + expect(test_results.evaluated_paths()).toEqual(['PUT /{index}']) + }) + + test('spec_paths', () => { + expect(test_results.spec_paths()).toEqual([ + "GET /_nodes/{id}", + "POST /_nodes/{id}", + "GET /cluster_manager", + "POST /cluster_manager", + "GET /index", + "GET /nodes" + ]) }) test('write_coverage', () => {