Added detailed test coverage report. (#513)

* Added detailed test coverage report.

Signed-off-by: dblock <[email protected]>

.github/pr-comment-templates/pr-test-coverage-analysis.template.md

@@ -1,8 +1,8 @@
 ## Spec Test Coverage Analysis
 {{with .test_coverage}}
 
-| Total             | Tested                                                   |
-|-------------------|----------------------------------------------------------|
-| {{.paths_count}}  | {{.evaluated_paths_count}} ({{.evaluated_paths_pct}} %)  |
+| Total                        | Tested                                                        |
+|------------------------------|---------------------------------------------------------------|
+| {{.total_operations_count}}  | {{.evaluated_operations_count}} ({{.evaluated_paths_pct}} %)  |
 
 {{end}}

CHANGELOG.md

@@ -77,6 +77,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Added `read_time`, `write_time`, `queue_size` and `io_time_in_millis` to `IoStatDevice` ([#483](https://github.com/opensearch-project/opensearch-api-specification/pull/483))
 - Added `total_rejections_breakup` to `ShardIndexingPressureStats` ([#483](https://github.com/opensearch-project/opensearch-api-specification/pull/483))
 - Added `cancelled_task_percentage` and `current_cancellation_eligible_tasks_count` to `ShardSearchBackpressureTaskCancellationStats` ([#483](https://github.com/opensearch-project/opensearch-api-specification/pull/483))
+- Added detailed test coverage report ([#513](https://github.com/opensearch-project/opensearch-api-specification/pull/513))
 
 ### Changed
 

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)
 <!-- TOC -->
 
 # Spec Testing Guide
@@ -318,3 +321,32 @@ The test runner may generate warnings that can be suppressed with `warnings:` at
   parameters:
     index: movies
 ```
+
+## Collecting Test Coverage
+
+### Coverage Summary
+
+The test tool can generate a test coverage summary using `--coverage <path>` with the number of evaluated verb + path combinations, a total number of paths and the % of paths tested. 
+
+```json
+{
+  "evaluated_operations_count": 214,
+  "operations_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}
+```

tools/src/tester/ChapterEvaluator.ts

@@ -84,6 +84,10 @@ export default class ChapterEvaluator {
 
     var result: ChapterEvaluation = {
       title: chapter.synopsis,
+      operation: {
+        method: chapter.method,
+        path: chapter.path
+      },
       path: `${chapter.method} ${chapter.path}`,
       overall: { result: overall_result(evaluations) },
       request: { parameters: params, request },

tools/src/tester/ResultLogger.ts

@@ -7,10 +7,11 @@
 * compatible open source license.
 */
 
-import { type ChapterEvaluation, type Evaluation, Result, type StoryEvaluation } from './types/eval.types'
+import { type ChapterEvaluation, type Evaluation, Operation, Result, type StoryEvaluation } from './types/eval.types'
 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_operations().length}/${results.operations().length} paths.`)
+  }
+
+  log_coverage_report(results: TestResults): void {
+    console.log()
+    console.log(`${results.unevaluated_operations().length} paths remaining.`)
+    const groups = _.groupBy(results.unevaluated_operations(), (operation) => operation.path.split('/')[1])
+    Object.entries(groups).forEach(([root, operations]) => {
+      this.#log_coverage_group(root, operations)
+    });
+  }
+
+  #log_coverage_group(key: string, operations: Operation[], index: number = 2): void {
+    if (operations.length == 0 || key == undefined) return
+    console.log(`${' '.repeat(index)}/${key} (${operations.length})`)
+    const current_level_operations = operations.filter((operation) => operation.path.split('/').length == index)
+    current_level_operations.forEach((operation) => {
+      console.log(`${' '.repeat(index + 2)}${operation.method} ${operation.path}`)
+    })
+    const next_level_operations = operations.filter((operation) => operation.path.split('/').length > index)
+    const subgroups = _.groupBy(next_level_operations, (operation) => operation.path.split('/')[index])
+    Object.entries(subgroups).forEach(([root, operations]) => {
+      this.#log_coverage_group(root, operations, index + 1)
+    });
   }
 
   #log_story ({ result, full_path, display_path, message, warnings }: StoryEvaluation): void {

tools/src/tester/StoryEvaluator.ts

@@ -99,8 +99,8 @@ export default class StoryEvaluator {
       return chapter.path
     }))
     const normalized_paths = _.map(paths, (path) => path.replaceAll(/\/\{[^}]+}/g, '').replaceAll('//', '/'))
-    const paths_counts: Record<string, number> = Object.assign((_.values(_.groupBy(normalized_paths)).map(p => { return { [p[0]] : p.length } })))
-    if (paths_counts.length > 1) {
+    const operations_counts: Record<string, number> = Object.assign((_.values(_.groupBy(normalized_paths)).map(p => { return { [p[0]] : p.length } })))
+    if (operations_counts.length > 1) {
       return `Multiple paths detected, please group similar tests together and move paths not being tested to prologues or epilogues.\n  ${_.join(_.uniq(paths), "\n  ")}\n`
     }
   }

tools/src/tester/TestResults.ts

@@ -9,35 +9,59 @@
 
 import _ from "lodash";
 import MergedOpenApiSpec from "./MergedOpenApiSpec";
-import { StoryEvaluations } from "./types/eval.types";
+import { Operation, 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
+  protected _evaluated_operations?: Operation[]
+  protected _unevaluated_operations?: Operation[]
+  protected _operations?: Operation[]
 
   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
+  evaluated_operations(): Operation[] {
+    if (this._evaluated_operations !== undefined) return this._evaluated_operations
+    this._evaluated_operations = _.uniq(_.compact(_.flatMap(this._evaluations.evaluations, (evaluation) =>
+      _.map(evaluation.chapters, (chapter) => chapter.operation)
+    )))
+    return this._evaluated_operations
   }
 
-  spec_paths_count(): number {
-    return Object.values(this._spec.paths()).reduce((acc, methods) => acc + methods.length, 0);
+  unevaluated_operations(): Operation[] {
+    if (this._unevaluated_operations !== undefined) return this._unevaluated_operations
+    this._unevaluated_operations = this.operations().filter((operation) =>
+      !_.find(this.evaluated_operations(),
+        (op) =>
+          operation.method == op.method &&
+          operation.path == op.path
+      )
+    )
+    return this._unevaluated_operations
+  }
+
+  operations(): Operation[] {
+    if (this._operations !== undefined) return this._operations
+    this._operations = _.uniq(Object.entries(this._spec.paths()).flatMap(([path, path_item]) => {
+      return Object.values(path_item).map((method) => {
+        return { method: method.toUpperCase(), path }
+      })
+    }))
+
+    return this._operations
   }
 
   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_operations_count: this.evaluated_operations().length,
+      total_operations_count: this.operations().length,
+      evaluated_paths_pct: this.operations().length > 0 ? Math.round(
+        this.evaluated_operations().length / this.operations().length * 100 * 100
       ) / 100 : 0,
     }
   }

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>', '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)

tools/src/tester/types/eval.types.ts

@@ -17,6 +17,11 @@ export interface StoryFile {
   story: Story
 }
 
+export interface Operation {
+  method: string
+  path: string
+}
+
 export interface StoryEvaluation {
   result: Result
   display_path: string
@@ -36,6 +41,7 @@ export interface StoryEvaluations {
 export interface ChapterEvaluation {
   title: string,
   overall: Evaluation,
+  operation?: Operation,
   path?: string,
   request?: {
     parameters?: Record<string, Evaluation>

tools/src/tester/types/test.types.ts

@@ -8,7 +8,7 @@
 */
 
 export interface SpecTestCoverage {
-  paths_count: number
-  evaluated_paths_count: number,
+  total_operations_count: number
+  evaluated_operations_count: number,
   evaluated_paths_pct: number
 }

tools/tests/tester/ResultLogger.test.ts

@@ -66,7 +66,11 @@ describe('ConsoleResultLogger', () => {
           {
             title: 'title',
             overall: { result: Result.PASSED },
-            path: 'path'
+            path: 'path',
+            operation: {
+              method: 'GET',
+              path: '/_nodes/{id}'
+            }
           }
         ]
       }] })
@@ -79,6 +83,45 @@ 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}',
+            operation: {
+              method: 'GET',
+              path: '/_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,

tools/tests/tester/TestResults.test.ts

@@ -17,38 +17,62 @@ describe('TestResults', () => {
 
   const evaluations = [{
     result: Result.PASSED,
-    display_path: 'path',
+    display_path: 'PUT /{index}',
     full_path: 'full_path',
     description: 'description',
     message: 'message',
     chapters: [{
       title: 'title',
+      operation: {
+        method: 'PUT',
+        path: '/{index}'
+      },
       overall: {
         result: Result.PASSED
       },
-      path: 'path'
+      path: 'PUT /{index}'
     }],
     epilogues: [],
     prologues: []
   }]
 
   const test_results = new TestResults(spec, { evaluations })
 
-  test('evaluated_paths_count', () => {
-    expect(test_results.evaluated_paths_count()).toEqual(1)
+  test('unevaluated_operations', () => {
+    expect(test_results.unevaluated_operations()).toEqual([
+      { method: "GET", path: "/_nodes/{id}" },
+      { method: "POST", path: "/_nodes/{id}" },
+      { method: "GET", path: "/cluster_manager" },
+      { method: "POST", path: "/cluster_manager" },
+      { method: "GET", path: "/index" },
+      { method: "GET", path: "/nodes" }
+    ])
+  })
+
+  test('evaluated_operations', () => {
+    expect(test_results.evaluated_operations()).toStrictEqual([
+      { method: 'PUT', path: '/{index}' }
+    ])
   })
 
-  test('spec_paths_count', () => {
-    expect(test_results.spec_paths_count()).toEqual(6)
+  test('operations', () => {
+    expect(test_results.operations()).toEqual([
+      { method: "GET", path: "/_nodes/{id}" },
+      { method: "POST", path: "/_nodes/{id}" },
+      { method: "GET", path: "/cluster_manager" },
+      { method: "POST", path: "/cluster_manager" },
+      { method: "GET", path: "/index" },
+      { method: "GET", path: "/nodes" }
+    ])
   })
 
   test('write_coverage', () => {
     const filename = 'coverage.json'
     test_results.write_coverage(filename)
     expect(JSON.parse(fs.readFileSync(filename, 'utf8'))).toEqual({
-      evaluated_paths_count: 1,
+      evaluated_operations_count: 1,
       evaluated_paths_pct: 16.67,
-      paths_count: 6
+      total_operations_count: 6
     })
     fs.unlinkSync(filename)
   })

tools/tests/tester/fixtures/evals/error/chapter_error.yaml

@@ -24,6 +24,9 @@ chapters:
     overall:
       result: ERROR
     path: DELETE /{index}
+    operation:
+      method: DELETE
+      path: /{index}
     request:
       parameters: {}
       request:

tools/tests/tester/fixtures/evals/error/output_error.yaml

@@ -11,6 +11,9 @@ chapters:
     overall:
       result: ERROR
     path: GET /_cat/health
+    operation:
+      method: GET
+      path: /_cat/health
     retries: 3
     request:
       parameters: