Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test coverage #10

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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}} |

{{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 @@ -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

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
47 changes: 47 additions & 0 deletions tools/src/tester/TestResults.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* 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 {
test_coverage: {
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())
}
}
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
15 changes: 15 additions & 0 deletions tools/src/tester/types/test.types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/*
* 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 {
test_coverage: {
paths_count: number
evaluated_paths_count: number
}
}
Loading
Loading