Skip to content

Commit

Permalink
Added rudimentary test coverage counting paths being tested. (#443)
Browse files Browse the repository at this point in the history
* Added test coverage.

Signed-off-by: dblock <[email protected]>
  • Loading branch information
dblock authored Jul 22, 2024
1 parent d8f1e02 commit 34ff292
Show file tree
Hide file tree
Showing 23 changed files with 380 additions and 44 deletions.
1 change: 1 addition & 0 deletions .cspell
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ aarch
actiongroup
actiongroups
aggregatable
argjson
asciifolding
authc
authinfo
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
## Spec Test Coverage Analysis
{{with .test_coverage}}

| Total | Tested |
|-------------------|----------------------------------------------------------|
| {{.paths_count}} | {{.evaluated_paths_count}} ({{.evaluated_paths_pct}} %) |

{{end}}
3 changes: 2 additions & 1 deletion .github/workflows/pr-comment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ on:
workflow_run:
workflows:
- Analyze PR Changes
- Test Spec
types:
- completed

Expand Down Expand Up @@ -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
edit-mode: replace
59 changes: 58 additions & 1 deletion .github/workflows/test-spec.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,61 @@ jobs:
run: docker-compose up -d

- name: Run Tests
run: npm run test:spec -- --opensearch-insecure
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: 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- 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 `plugins` to NodeInfoSettings ([#442](https://github.com/opensearch-project/opensearch-api-specification/pull/442))
- Added test coverage ([#443](https://github.com/opensearch-project/opensearch-api-specification/pull/443))

### Changed

Expand Down
1 change: 1 addition & 0 deletions tools/src/tester/ChapterEvaluator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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] : [])) },
Expand Down
13 changes: 12 additions & 1 deletion tools/src/tester/MergedOpenApiSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -37,6 +38,16 @@ export default class MergedOpenApiSpec {
return (this.spec().info as any)['x-api-version']
}

paths(): Record<string, string[]> {
var obj: Record<string, string[]> = {}
_.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
Expand Down
8 changes: 8 additions & 0 deletions tools/src/tester/ResultLogger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,16 @@
import { type ChapterEvaluation, type Evaluation, Result, type StoryEvaluation } from './types/eval.types'
import { overall_result } from './helpers'
import * as ansi from './Ansi'
import TestResults from './TestResults'

export interface ResultLogger {
log: (evaluation: StoryEvaluation) => void
log_coverage: (_results: TestResults) => void
}

export class NoOpResultLogger implements ResultLogger {
log (_: StoryEvaluation): void { }
log_coverage(_results: TestResults): void { }
}

export class ConsoleResultLogger implements ResultLogger {
Expand All @@ -38,6 +41,11 @@ export class ConsoleResultLogger implements ResultLogger {
if (with_padding) console.log()
}

log_coverage(results: TestResults): void {
console.log()
console.log(`Tested ${results.evaluated_paths_count()}/${results.spec_paths_count()} 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)))
}
Expand Down
10 changes: 5 additions & 5 deletions tools/src/tester/StoryEvaluator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<StoryEvaluation> {
if (story.version !== undefined && !semver.satisfies(version, story.version)) {
async evaluate({ story, display_path, full_path }: StoryFile, version?: string, dry_run: boolean = false): Promise<StoryEvaluation> {
if (version != undefined && story.version !== undefined && !semver.satisfies(version, story.version)) {
return {
result: Result.SKIPPED,
display_path,
Expand All @@ -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,
Expand All @@ -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<ChapterEvaluation[]> {
async #evaluate_chapters(chapters: Chapter[], has_errors: boolean, dry_run: boolean, story_outputs: StoryOutputs, version?: string): Promise<ChapterEvaluation[]> {
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 {
Expand Down
48 changes: 48 additions & 0 deletions tools/src/tester/TestResults.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* 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";
import { SpecTestCoverage } from "./types/test.types";
import { write_json } from "../helpers";

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);
}

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
) / 100 : 0,
}
}

write_coverage(file_path: string): void {
write_json(file_path, this.test_coverage())
}
}
13 changes: 7 additions & 6 deletions tools/src/tester/TestRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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()
Expand All @@ -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[] {
Expand Down
12 changes: 11 additions & 1 deletion tools/src/tester/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.')
Expand All @@ -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>', 'path to write test coverage results to'))
.allowExcessArguments(false)
.parse()

Expand All @@ -62,7 +64,15 @@ const runner = new TestRunner(http_client, story_validator, story_evaluator, res

runner.run(opts.testsPath, spec.api_version(), opts.dryRun)
.then(
({ failed }) => {
({ 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 })
9 changes: 7 additions & 2 deletions tools/src/tester/types/eval.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, Evaluation>
request_body?: Evaluation
Expand Down
14 changes: 14 additions & 0 deletions tools/src/tester/types/test.types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/*
* 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,
evaluated_paths_pct: number
}
Loading

0 comments on commit 34ff292

Please sign in to comment.