Warn if file path is invalid.

Signed-off-by: dblock <[email protected]>
opensearch-project · Jan 16, 2025 · 953fa72 · 953fa72
1 parent 639fd57
commit 953fa72
Show file tree

Hide file tree

Showing 13 changed files with 73 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -52,6 +52,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Added `GET /_plugins/_ml/profile`, `GET /_plugins/_ml/profile/models`, `models/{model_id}`, `tasks`, `tasks/{task_id}` ([#787](https://github.com/opensearch-project/opensearch-api-specification/pull/787))
 - Added `GET /_plugins/_ml/stats/`, `stats/{stat}`, `{nodeId}/stats/`, `{nodeId}/stats/{stat}` ([#794](https://github.com/opensearch-project/opensearch-api-specification/pull/794))
 - Added `GET`, `POST /_plugins/_ml/tasks/_search`, `GET /_plugins/_ml/tools`, `tools/{tool_name}` ([#797](https://github.com/opensearch-project/opensearch-api-specification/pull/797))
+- Added a warning for test file names that don't match the API being tested ([#793](https://github.com/opensearch-project/opensearch-api-specification/pull/793))
 
 ### Removed
 - Removed unsupported `_common.mapping:SourceField`'s `mode` field and associated `_common.mapping:SourceFieldMode` enum ([#652](https://github.com/opensearch-project/opensearch-api-specification/pull/652))

diff --git a/TESTING_GUIDE.md b/TESTING_GUIDE.md
@@ -20,6 +20,7 @@
     - [Waiting for Tasks](#waiting-for-tasks)
     - [Warnings](#warnings)
       - [multiple-paths-detected](#multiple-paths-detected)
+      - [invalid-path-detected](#invalid-path-detected)
       - [Suppressing Warnings](#suppressing-warnings)
   - [Collecting Test Coverage](#collecting-test-coverage)
     - [Coverage Summary](#coverage-summary)
@@ -367,6 +368,15 @@ WARNING Multiple paths detected, please group similar tests together and move pa
   /{index}
 ```
 
+#### invalid-path-detected
+
+The test file names expect to match the API being tested. Otherwise, a warning will be emitted.
+
+```
+PASSED  _core/rank_eval.yaml (tests/default/_core/rank_eval.yaml)
+WARNING Invalid path detected, please move /tests/default/_core/rank_eval.yaml to /rankeval.yaml.
+```
+
 #### Suppressing Warnings
 
 The test runner may generate warnings that can be suppressed with `warnings:` at story or chapter level. For example, to suppress the multiple paths detected warning.

diff --git a/json_schemas/test_story.schema.yaml b/json_schemas/test_story.schema.yaml
@@ -252,4 +252,8 @@ definitions:
         type: boolean
         default: true
         description: Enable/disable warnings about multiple paths being tested in the same story.
+      invalid-path-detected:
+        type: boolean
+        default: true
+        description: Enable/disable warnings about file paths that do not match paths tested in the story.
     additionalProperties: false
diff --git a/tests/default/_core/info.yaml b/tests/default/_core/info.yaml
@@ -2,6 +2,8 @@
 $schema: ../../../json_schemas/test_story.schema.yaml
 
 description: Test root endpoint.
+warnings:
+  invalid-path-detected: false
 distributions:
   excluded:
     - amazon-serverless

diff --git a/tools/src/tester/StoryEvaluator.ts b/tools/src/tester/StoryEvaluator.ts
@@ -18,6 +18,7 @@ import * as semver from '../_utils/semver'
 import _ from 'lodash'
 import { ParsedChapter, ParsedStory } from './types/parsed_story.types'
 import { OutputReference } from './OutputReference'
+import * as path from 'path'
 
 export default class StoryEvaluator {
   private readonly _chapter_evaluator: ChapterEvaluator
@@ -79,18 +80,19 @@ export default class StoryEvaluator {
       result: overall_result(prologues.concat(chapters).concat(epilogues).concat(prologues).map(e => e.overall)),
     }
 
-    const warnings = this.#chapter_warnings(story)
+    const warnings = this.#chapter_warnings(story, full_path)
     if (warnings !== undefined) {
       result.warnings = warnings
     }
 
     return result
   }
 
-  #chapter_warnings(story: ParsedStory): string[] | undefined {
-    const result = _.compact([
-      this.#warning_if_mismatched_chapter_paths(story)
-    ])
+  #chapter_warnings(story: ParsedStory, full_path: string): string[] | undefined {
+    const result = _.compact(_.flattenDeep([
+      [this.#warning_if_mismatched_chapter_paths(story)],
+      this.#warning_if_invalid_path(story, full_path)
+    ]))
     return result.length > 0 ? result : undefined
   }
 
@@ -103,10 +105,33 @@ export default class StoryEvaluator {
     const normalized_paths = _.map(paths, (path) => path.replaceAll(/\/\{[^}]+}/g, '').replaceAll('//', '/'))
     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`
+      return `Multiple paths detected, please group similar tests together and move paths not being tested to prologues or epilogues.\n  ${_.join(_.uniq(paths), "\n  ")}`
     }
   }
 
+  #warning_if_invalid_path(story: ParsedStory, full_path: string): string[] | undefined {
+    if (story.warnings?.['invalid-path-detected'] === false) return
+    const paths = _.compact(_.map(story.chapters, (chapter) => {
+      if (chapter.warnings?.['multiple-paths-detected'] === false) return // not the path being tested
+      return chapter.path
+    }))
+    const normalized_paths = _.uniq(_.map(paths, (path) =>
+      path
+        .replaceAll('/_plugins/', '')
+        .replaceAll(/\/\{[^}]+}/g, '')
+        .replaceAll('//', '/')
+        .replaceAll('_', '')
+      + '.yaml'
+    ))
+
+    return _.compact(_.map(normalized_paths, (normalized_path) => {
+      if (!full_path.endsWith(normalized_path)) {
+        const relative_path = path.relative('.', full_path)
+        return `Invalid path detected, please move /${relative_path} to ${normalized_path}.`
+      }
+    }))
+  }
+
   async #evaluate_chapters(chapters: ParsedChapter[], has_errors: boolean, dry_run: boolean, story_outputs: StoryOutputs, version?: string, distribution?: string): Promise<ChapterEvaluation[]> {
     const evaluations: ChapterEvaluation[] = []
     for (const chapter of chapters) {

diff --git a/tools/src/tester/types/story.types.ts b/tools/src/tester/types/story.types.ts
@@ -202,6 +202,10 @@ export interface Warnings {
    * Enable/disable warnings about multiple paths being tested in the same story.
    */
   'multiple-paths-detected'?: boolean;
+  /**
+   * Enable/disable warnings about file paths that do not match paths tested in the story.
+   */
+  'invalid-path-detected'?: boolean;
 }
 /**
  * This interface was referenced by `Story`'s JSON-Schema

diff --git a/tools/tests/tester/StoryValidator.test.ts b/tools/tests/tester/StoryValidator.test.ts
@@ -31,10 +31,11 @@ describe('StoryValidator', () => {
     const evaluation = validate('tools/tests/tester/fixtures/invalid_story.yaml')
     expect(evaluation?.result).toBe('ERROR')
     expect(evaluation?.message).toBe("Invalid Story: " +
-      "data/epilogues/0 contains unsupported properties: response --- " +
       "data/chapters/0 MUST contain the missing properties: method --- " +
       "data/chapters/1/method MUST be equal to one of the allowed values: GET, PUT, POST, DELETE, PATCH, HEAD, OPTIONS --- " +
-      "data/chapters/1/method must be array --- data/chapters/1/method must match exactly one schema in oneOf")
+      "data/chapters/1/method must be array --- " +
+      "data/chapters/1/method must match exactly one schema in oneOf"
+    )
   })
 
   test('invalid description', () => {

diff --git a/tools/tests/tester/fixtures/evals/error/chapter_error.yaml b/tools/tests/tester/fixtures/evals/error/chapter_error.yaml
@@ -1,15 +1,18 @@
+# eslint-disable yml/sort-sequence-values
 display_path: error/chapter_error.yaml
 full_path: tools/tests/tester/fixtures/stories/error/chapter_error.yaml
 
 result: ERROR
 description: This story should failed due to missing info in the spec.
 
 warnings:
-  - |
+  - >-
     Multiple paths detected, please group similar tests together and move paths not being tested to prologues or epilogues.
       /{index}/settings
       /{index}
       /_cat/indices
+  - Invalid path detected, please move /tools/tests/tester/fixtures/stories/error/chapter_error.yaml to /settings.yaml.
+  - Invalid path detected, please move /tools/tests/tester/fixtures/stories/error/chapter_error.yaml to /cat/indices.yaml.
 prologues:
   - title: PUT /books
     overall:

diff --git a/tools/tests/tester/fixtures/evals/error/output_error.yaml b/tools/tests/tester/fixtures/evals/error/output_error.yaml
@@ -2,6 +2,8 @@ display_path: error/output_error.yaml
 full_path: tools/tests/tester/fixtures/stories/error/output_error.yaml
 
 result: ERROR
+warnings: 
+  - Invalid path detected, please move /tools/tests/tester/fixtures/stories/error/output_error.yaml to /cat/health.yaml.
 description: This story has an error in the output.
 prologues: []
 epilogues: []

diff --git a/tools/tests/tester/fixtures/evals/error/prologue_error.yaml b/tools/tests/tester/fixtures/evals/error/prologue_error.yaml
@@ -2,6 +2,9 @@ display_path: error/prologue_error.yaml
 full_path: tools/tests/tester/fixtures/stories/error/prologue_error.yaml
 
 result: ERROR
+warnings: 
+  - Invalid path detected, please move /tools/tests/tester/fixtures/stories/error/prologue_error.yaml to /cat/health.yaml.
+  - Invalid path detected, please move /tools/tests/tester/fixtures/stories/error/prologue_error.yaml to /cat/indices.yaml.
 description: This story should failed due to missing info in the spec.
 prologues:
   - title: PUT /books

diff --git a/tools/tests/tester/fixtures/evals/failed/not_found.yaml b/tools/tests/tester/fixtures/evals/failed/not_found.yaml
@@ -1,14 +1,16 @@
+# eslint-disable yml/sort-sequence-values
 display_path: failed/not_found.yaml
 full_path: tools/tests/tester/fixtures/stories/failed/not_found.yaml
 
 result: FAILED
 description: This story should failed due to missing info in the spec.
 prologues: []
 warnings:
-  - |
+  - >-
     Multiple paths detected, please group similar tests together and move paths not being tested to prologues or epilogues.
       /_cat/does_not_exist
       /{index}
+  - Invalid path detected, please move /tools/tests/tester/fixtures/stories/failed/not_found.yaml to /cat/doesnotexist.yaml.
 chapters:
   - title: This chapter should fail because the operation is not defined in the spec.
     overall:

diff --git a/tools/tests/tester/fixtures/evals/passed/passed.yaml b/tools/tests/tester/fixtures/evals/passed/passed.yaml
@@ -2,6 +2,9 @@ display_path: passed/passed.yaml
 full_path: tools/tests/tester/fixtures/stories/passed/passed.yaml
 
 result: PASSED
+warnings:
+  - Invalid path detected, please move /tools/tests/tester/fixtures/stories/passed/passed.yaml to /cat.yaml.
+  - Invalid path detected, please move /tools/tests/tester/fixtures/stories/passed/passed.yaml to /cat/health.yaml.
 description: This story should pass.
 prologues: []
 chapters:

diff --git a/tools/tests/tester/fixtures/evals/passed/value_type.yaml b/tools/tests/tester/fixtures/evals/passed/value_type.yaml
@@ -1,13 +1,15 @@
+# eslint-disable yml/sort-sequence-values
 display_path: passed/value_type.yaml
 full_path: tools/tests/tester/fixtures/stories/passed/value_type.yaml
 
 result: PASSED
 description: This story has an error trying to reuse a variable of a different type.
 warnings: 
-  - |
+  - >-
     Multiple paths detected, please group similar tests together and move paths not being tested to prologues or epilogues.
       /_cat/health
       /{index}
+  - Invalid path detected, please move /tools/tests/tester/fixtures/stories/passed/value_type.yaml to /cat/health.yaml.
 prologues:
   - overall:
       result: PASSED