Validate test story descriptions. (opensearch-project#467)

* Refactored validate_description and title into a base class. Signed-off-by: dblock <[email protected]> * Validate story descriptions. Signed-off-by: dblock <[email protected]> * Use a pattern to validate story synopsis and description. Signed-off-by: dblock <[email protected]> --------- Signed-off-by: dblock <[email protected]>
Tokesh · Aug 5, 2024 · c7dbaaf · c7dbaaf
1 parent 21bd106
commit c7dbaaf
Show file tree

Hide file tree

Showing 21 changed files with 59 additions and 41 deletions.
diff --git a/json_schemas/test_story.schema.yaml b/json_schemas/test_story.schema.yaml
@@ -6,6 +6,7 @@ properties:
     type: string
   description:
     type: string
+    pattern: ^\p{Lu}[\s\S]*\.$
   prologues:
     type: array
     items:
@@ -32,6 +33,7 @@ definitions:
           synopsis:
             type: string
             description: A brief description of the chapter.
+            pattern: ^\p{Lu}[\s\S]*\.$
           response:
             $ref: '#/definitions/ExpectedResponse'
           warnings:

diff --git a/tests/_core/reindex/pipeline.yaml b/tests/_core/reindex/pipeline.yaml
@@ -16,7 +16,7 @@ prologues:
     method: PUT
     request:
       payload:
-        description: |
+        description: |-
           Splits the `title`` field into a `words` list.
           Computes the length of the title field and stores it in a new `length` field.
           Removes the `year` field.

diff --git a/tests/indices/aliases/put_alias.yaml b/tests/indices/aliases/put_alias.yaml
@@ -63,7 +63,7 @@ chapters:
       payload:
         acknowledged: true
 
-  - synopsis: Retrieve aliases
+  - synopsis: Retrieve aliases.
     path: /{index}/_alias
     method: GET
     parameters:

diff --git a/tests/ingest/pipeline.yaml b/tests/ingest/pipeline.yaml
@@ -1,7 +1,6 @@
 $schema: ../../json_schemas/test_story.schema.yaml
 
-description: |
-  Test the creation of an ingest pipeline with a text embedding processor.
+description: Test the creation of an ingest pipeline with a text embedding processor.
 epilogues:
   - path: /_ingest/pipeline/books_pipeline
     method: DELETE

diff --git a/tests/ml/model_groups.yaml b/tests/ml/model_groups.yaml
@@ -1,7 +1,6 @@
 $schema: ../../json_schemas/test_story.schema.yaml
 
-description: |
-  Test the creation of model groups.
+description: Test the creation of model groups.
 version: '>= 2.11'
 prologues:
   - path: /_cluster/settings
@@ -28,7 +27,7 @@ chapters:
     request:
       payload:
         name: NLP_Group
-        description: Model group for NLP models
+        description: Model group for NLP models.
     response:
       status: 200
     output:

diff --git a/tests/ml/models.yaml b/tests/ml/models.yaml
@@ -1,7 +1,6 @@
 $schema: ../../json_schemas/test_story.schema.yaml
 
-description: |
-  Test the creation of models.
+description: Test the creation of models.
 version: '>= 2.11'
 prologues:
   - path: /_cluster/settings

diff --git a/tests/ppl/explain.yaml b/tests/ppl/explain.yaml
@@ -12,7 +12,7 @@ epilogues:
     method: DELETE
     status: [200, 404]
 chapters:
-  - synopsis: Get explain of SQL Query
+  - synopsis: Get explain of SQL Query.
     path: /_plugins/_ppl/_explain
     method: POST
     request:

diff --git a/tests/ppl/query.yaml b/tests/ppl/query.yaml
@@ -14,7 +14,7 @@ epilogues:
     method: DELETE
     status: [200, 404]
 chapters:
-  - synopsis: Get PPL query
+  - synopsis: Get PPL query.
     path: /_plugins/_ppl
     method: POST
     request:

diff --git a/tests/sql/close.yaml b/tests/sql/close.yaml
@@ -1,6 +1,6 @@
 $schema: ../../json_schemas/test_story.schema.yaml
 
-description: Test to explicitly clear the cursor context
+description: Test to explicitly clear the cursor context.
 
 prologues:
   - path: /{index}

diff --git a/tests/sql/explain.yaml b/tests/sql/explain.yaml
@@ -18,7 +18,7 @@ epilogues:
     method: DELETE
     status: [200, 404]
 chapters:
-  - synopsis: Get explain of SQL Query
+  - synopsis: Get explain of SQL Query.
     path: /_plugins/_sql/_explain
     method: POST
     request:

diff --git a/tests/sql/query.yaml b/tests/sql/query.yaml
@@ -20,7 +20,7 @@ epilogues:
     method: DELETE
     status: [200, 404]
 chapters:
-  - synopsis: Get SQL query
+  - synopsis: Get SQL query.
     path: /_plugins/_sql
     method: POST
     parameters:

diff --git a/tools/src/linter/components/Info.ts b/tools/src/linter/components/Info.ts
@@ -10,9 +10,6 @@
 import { type OpenAPIV3 } from 'openapi-types'
 import { type ValidationError } from 'types'
 import ValidatorBase from './base/ValidatorBase'
-import { toLaxTitleCase } from 'titlecase'
-
-const DESCRIPTION_REGEX = /^\p{Lu}[\s\S]*\.$/u
 
 export default class Info extends ValidatorBase {
   path: string
@@ -32,16 +29,10 @@ export default class Info extends ValidatorBase {
   }
 
   validate_description (): ValidationError | undefined {
-    const description = this.spec?.description ?? ''
-    if (description === '') { return this.error('Missing description property.') }
-    if (!DESCRIPTION_REGEX.test(description)) { return this.error('Description must start with a capital letter and end with a period.') }
+    return this.validate_description_field(this.spec?.description, true)
   }
 
   validate_title (): ValidationError | undefined {
-    const title = this.spec?.title ?? ''
-    if (title === '') { return this.error('Missing title property.') }
-    const expected_title = toLaxTitleCase(title)
-    if (title.endsWith('.')) { return this.error('Title must not end with a period.') }
-    if (title !== expected_title) return this.error(`Title must be capitalized, expected '${expected_title}'.`)
+    return this.validate_title_field(this.spec?.title)
   }
 }
diff --git a/tools/src/linter/components/Operation.ts b/tools/src/linter/components/Operation.ts
@@ -12,7 +12,6 @@ import _ from 'lodash'
 import ValidatorBase from './base/ValidatorBase'
 
 const GROUP_REGEX = /^([a-z]+[a-z_]*[a-z]+\.)?([a-z]+[a-z_]*[a-z]+)$/
-const DESCRIPTION_REGEX = /^\p{Lu}[\s\S]*\.$/u
 
 export default class Operation extends ValidatorBase {
   path: string
@@ -65,9 +64,7 @@ export default class Operation extends ValidatorBase {
   }
 
   validate_description (): ValidationError | undefined {
-    const description = this.spec.description ?? ''
-    if (description === '') { return this.error('Missing description property.') }
-    if (!DESCRIPTION_REGEX.test(description)) return this.error('Description must start with a capital letter and end with a period.')
+    return this.validate_description_field(this.spec?.description, true)
   }
 
   validate_operation_id (): ValidationError | undefined {

diff --git a/tools/src/linter/components/Schema.ts b/tools/src/linter/components/Schema.ts
@@ -12,7 +12,6 @@ import { type OpenAPIV3 } from 'openapi-types'
 import { type ValidationError } from 'types'
 
 const NAME_REGEX = /^[A-Za-z0-9]+$/
-const DESCRIPTION_REGEX = /^\p{Lu}[\s\S]*\.$/u
 
 export default class Schema extends ValidatorBase {
   name: string
@@ -36,8 +35,6 @@ export default class Schema extends ValidatorBase {
   }
 
   validate_description (): ValidationError | undefined {
-    const description = this.spec.description ?? ''
-    if (description === '') return
-    if (!DESCRIPTION_REGEX.test(description)) { return this.error('Description must start with a capital letter and end with a period.') }
+    return this.validate_description_field(this.spec.description)
   }
 }
diff --git a/tools/src/linter/components/base/ValidatorBase.ts b/tools/src/linter/components/base/ValidatorBase.ts
@@ -7,7 +7,11 @@
 * compatible open source license.
 */
 
+import { toLaxTitleCase } from 'titlecase'
 import { type ValidationError } from 'types'
+
+const DESCRIPTION_REGEX = /^\p{Lu}[\s\S]*\.$/u
+
 export default class ValidatorBase {
   file: string
   location: string | undefined
@@ -24,4 +28,18 @@ export default class ValidatorBase {
   validate (): ValidationError[] {
     throw new Error('Method not implemented.')
   }
+
+  validate_description_field (value?: string, required: boolean = false, key: string = 'description'): ValidationError | undefined {
+    if (value === undefined) { return required ? this.error(`Missing ${key} property.`) : undefined }
+    if (value === '') { return this.error(`Empty ${key} property.`) }
+    if (!DESCRIPTION_REGEX.test(value)) { return this.error(`The ${key} must start with a capital letter and end with a period, got "${value}".`) }
+  }
+
+  validate_title_field (value?: string, required: boolean = false, key: string = 'title'): ValidationError | undefined {
+    if (value === undefined) { return required ? this.error(`Missing ${key} property.`) : undefined }
+    if (value === '') { return this.error(`Empty ${key} property.`) }
+    const expected = toLaxTitleCase(value)
+    if (value.endsWith('.')) { return this.error(`The ${key} must not end with a period.`) }
+    if (value !== expected) return this.error(`The ${key} must be capitalized, expected "${expected}", not "${value}".`)
+  }
 }
diff --git a/tools/tests/linter/NamespaceFile.test.ts b/tools/tests/linter/NamespaceFile.test.ts
@@ -105,12 +105,12 @@ test('validate_info() periods', () => {
     {
       file: 'namespaces/invalid_info_periods.yaml',
       location: 'Info',
-      message: 'Title must not end with a period.'
+      message: 'The title must not end with a period.'
     },
     {
       file: 'namespaces/invalid_info_periods.yaml',
       location: 'Info',
-      message: 'Description must start with a capital letter and end with a period.'
+      message: 'The description must start with a capital letter and end with a period, got "Description should have a period".'
     }
   ])
 })
@@ -126,12 +126,12 @@ test('validate_info() capitals', () => {
     {
       file: 'namespaces/invalid_info_capitals.yaml',
       location: 'Info',
-      message: "Title must be capitalized, expected 'Title Must Be Capitalized'."
+      message: "The title must be capitalized, expected \"Title Must Be Capitalized\", not \"Title must be capitalized\"."
     },
     {
       file: 'namespaces/invalid_info_capitals.yaml',
       location: 'Info',
-      message: "Description must start with a capital letter and end with a period."
+      message: "The description must start with a capital letter and end with a period, got \"description must start with a capital letter and end with a period.\"."
     }
   ])
 })

diff --git a/tools/tests/linter/Operation.test.ts b/tools/tests/linter/Operation.test.ts
@@ -69,7 +69,7 @@ test('validate_description()', () => {
 
   const invalid_description = operation({ 'x-operation-group': 'indices.create', description: 'This is a description without a period' })
   expect(invalid_description.validate_description())
-    .toEqual(invalid_description.error('Description must start with a capital letter and end with a period.'))
+    .toEqual(invalid_description.error('The description must start with a capital letter and end with a period, got "This is a description without a period".'))
 
   const valid_description = operation({ 'x-operation-group': 'indices.create', description: 'This is a description with a period.' })
   expect(valid_description.validate_description())

diff --git a/tools/tests/linter/Schema.test.ts b/tools/tests/linter/Schema.test.ts
@@ -26,6 +26,6 @@ test('validate_description()', () => {
   expect(schema('Name', { description: 'Does not end with a period' }).validate_description()).toEqual({
     file: '_common.yaml',
     location: '#/components/schemas/Name',
-    message: "Description must start with a capital letter and end with a period."
+    message: "The description must start with a capital letter and end with a period, got \"Does not end with a period\"."
   })
 })
diff --git a/tools/tests/tester/StoryValidator.test.ts b/tools/tests/tester/StoryValidator.test.ts
@@ -35,6 +35,14 @@ describe('StoryValidator', () => {
       "data/chapters/1/method MUST be equal to one of the allowed values: GET, PUT, POST, DELETE, PATCH, HEAD, OPTIONS")
   })
 
+  test('invalid description', () => {
+    const evaluation = validate('tools/tests/tester/fixtures/invalid_description.yaml')
+    expect(evaluation?.result).toBe('ERROR')
+    expect(evaluation?.message).toBe("Invalid Story: " +
+      "data/description must match pattern \"^\\p{Lu}[\\s\\S]*\\.$\" --- " +
+      "data/chapters/0/synopsis must match pattern \"^\\p{Lu}[\\s\\S]*\\.$\"")
+  })
+
   test('valid story', () => {
     const evaluation = validate('tools/tests/tester/fixtures/valid_story.yaml')
     expect(evaluation).toBeUndefined()

diff --git a/tools/tests/tester/fixtures/invalid_description.yaml b/tools/tests/tester/fixtures/invalid_description.yaml
@@ -0,0 +1,8 @@
+$schema: ../../../../json_schemas/test_story.schema.yaml
+
+description: This story description is missing a period
+
+chapters:
+  - synopsis: this synopsis is not capitalized.
+    method: GET
+    path: /_cat/health
diff --git a/tools/tests/tester/fixtures/invalid_story.yaml b/tools/tests/tester/fixtures/invalid_story.yaml
@@ -2,9 +2,9 @@ $schema: ../../../../json_schemas/test_story.schema.yaml
 
 description: This story is invalid when validated against test_story.schema.yaml.
 chapters:
-  - synopsis: Missing Method
+  - synopsis: Missing method.
     path: /{index}
-  - synopsis: Invalid Method
+  - synopsis: Invalid method.
     path: /
     method: RETRIEVE
 epilogues: