Fix: evaluate output values before schema checking. (#536)

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

tools/src/tester/ChapterEvaluator.ts

@@ -63,8 +63,8 @@ export default class ChapterEvaluator {
 
   async #evaluate(chapter: Chapter, operation: ParsedOperation, story_outputs: StoryOutputs, retries?: number): Promise<ChapterEvaluation> {
     const response = await this._chapter_reader.read(chapter, story_outputs)
-    const params = this.#evaluate_parameters(chapter, operation)
-    const request = this.#evaluate_request(chapter, operation)
+    const params = this.#evaluate_parameters(chapter, operation, story_outputs)
+    const request = this.#evaluate_request(chapter, operation, story_outputs)
     const status = this.#evaluate_status(chapter, response)
     const payload_schema_evaluation = status.result === Result.PASSED ? this.#evaluate_payload_schema(chapter, response, operation) : { result: Result.SKIPPED }
     const output_values_evaluation: EvaluationWithOutput = status.result === Result.PASSED ? ChapterOutput.extract_output_values(response, chapter.output) : { evaluation: { result: Result.SKIPPED } }
@@ -110,21 +110,23 @@ export default class ChapterEvaluator {
     return result
   }
 
-  #evaluate_parameters(chapter: Chapter, operation: ParsedOperation): Record<string, Evaluation> {
-    return Object.fromEntries(Object.entries(chapter.parameters ?? {}).map(([name, parameter]) => {
+  #evaluate_parameters(chapter: Chapter, operation: ParsedOperation, story_outputs: StoryOutputs): Record<string, Evaluation> {
+    const parameters: Record<string, any> = story_outputs.resolve_value(chapter.parameters) ?? {}
+    return Object.fromEntries(Object.entries(parameters).map(([name, parameter]) => {
       const schema = operation.parameters[name]?.schema
       if (schema == null) return [name, { result: Result.FAILED, message: `Schema for "${name}" parameter not found.` }]
       const evaluation = this._schema_validator.validate(schema, parameter)
       return [name, evaluation]
     }))
   }
 
-  #evaluate_request(chapter: Chapter, operation: ParsedOperation): Evaluation {
+  #evaluate_request(chapter: Chapter, operation: ParsedOperation, story_outputs: StoryOutputs): Evaluation {
     if (chapter.request?.payload === undefined) return { result: Result.PASSED }
     const content_type = chapter.request.content_type ?? APPLICATION_JSON
     const schema = operation.requestBody?.content[content_type]?.schema
     if (schema == null) return { result: Result.FAILED, message: `Schema for "${content_type}" request body not found in the spec.` }
-    return this._schema_validator.validate(schema, chapter.request?.payload ?? {})
+    const payload = story_outputs.resolve_value(chapter.request?.payload) ?? {}
+    return this._schema_validator.validate(schema, payload)
   }
 
   #evaluate_status(chapter: Chapter, response: ActualResponse): Evaluation {

tools/tests/tester/fixtures/evals/passed.yaml → tools/tests/tester/fixtures/evals/passed/passed.yaml

@@ -1,5 +1,5 @@
-display_path: passed.yaml
-full_path: tools/tests/tester/fixtures/stories/passed.yaml
+display_path: passed/passed.yaml
+full_path: tools/tests/tester/fixtures/stories/passed/passed.yaml
 
 result: PASSED
 description: This story should pass.

tools/tests/tester/fixtures/evals/passed/value_type.yaml

@@ -0,0 +1,68 @@
+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}
+prologues:
+  - overall:
+      result: PASSED
+    title: GET /
+    output:
+      outputs:
+        boolean: false
+epilogues:
+  - overall:
+      result: PASSED
+    title: DELETE /movies
+chapters:
+  - title: This chapter requires a boolean in the parameters.
+    overall:
+      result: PASSED
+    path: GET /_cat/health
+    operation:
+      method: GET
+      path: /_cat/health
+    request:
+      parameters:
+        format:
+          result: PASSED
+        v:
+          result: PASSED
+      request:
+        result: PASSED
+    response:
+      status:
+        result: PASSED
+      payload_body:
+        result: PASSED
+      payload_schema:
+        result: PASSED
+      output_values:
+        result: SKIPPED
+  - title: This chapter requires a boolean in the body.
+    overall:
+      result: PASSED
+    path: PUT /{index}
+    operation:
+      method: PUT
+      path: /{index}
+    request:
+      parameters:
+        index:
+          result: PASSED
+      request:
+        result: PASSED
+    response:
+      status:
+        result: PASSED
+      payload_body:
+        result: PASSED
+      payload_schema:
+        result: PASSED
+      output_values:
+        result: SKIPPED

tools/tests/tester/fixtures/specs/excerpt.yaml

@@ -22,6 +22,7 @@ paths:
       description: Returns health for the Cat APIs.
       parameters:
         - $ref: '#/components/parameters/cat.health::query.format'
+        - $ref: '#/components/parameters/cat.health::query.v'
       responses:
         '200':
           $ref: '#/components/responses/cat.health@200'
@@ -40,6 +41,20 @@ paths:
         '200':
           $ref: '#/components/responses/cat.indices@200'
   /{index}:
+    put:
+      operationId: indices.create.0
+      x-operation-group: indices.create
+      x-version-added: '1.0'
+      description: Creates an index with optional settings and mappings.
+      externalDocs:
+        url: https://opensearch.org/docs/latest/api-reference/index-apis/create-index/
+      parameters:
+        - $ref: '#/components/parameters/indices.create::path.index'
+      requestBody:
+        $ref: '#/components/requestBodies/indices.create'
+      responses:
+        '200':
+          $ref: '#/components/responses/indices.create@200'
     delete:
       operationId: indices.delete.0
       x-operation-group: indices.delete
@@ -65,20 +80,6 @@ paths:
       responses:
         '200':
           $ref: '#/components/responses/indices.exists@200'
-    put:
-      operationId: indices.create.0
-      x-operation-group: indices.create
-      x-version-added: '1.0'
-      description: Creates an index with optional settings and mappings.
-      externalDocs:
-        url: https://opensearch.org/docs/latest/api-reference/index-apis/create-index/
-      parameters:
-        - $ref: '#/components/parameters/indices.create::path.index'
-      requestBody:
-        $ref: '#/components/requestBodies/indices.create'
-      responses:
-        '200':
-          $ref: '#/components/responses/indices.create@200'
 components:
   requestBodies:
     indices.create:
@@ -90,6 +91,12 @@ components:
               settings:
                 type: object
                 description: Optional settings for the index.
+                properties:
+                  shard:
+                    type: object
+                    properties:
+                      check_on_startup:
+                        type: boolean
               mappings:
                 type: object
                 description: Optional mappings for the index.
@@ -180,6 +187,11 @@ components:
       name: format
       schema:
         type: string
+    cat.health::query.v:
+      in: query
+      name: v
+      schema:
+        type: boolean
     cat.help::query.format:
       in: query
       name: format

tools/tests/tester/fixtures/stories/passed.yaml → tools/tests/tester/fixtures/stories/passed/passed.yaml

@@ -1,4 +1,4 @@
-$schema: ../../../../../json_schemas/test_story.schema.yaml
+$schema: ../../../../../../json_schemas/test_story.schema.yaml
 
 description: This story should pass.
 warnings:

tools/tests/tester/fixtures/stories/passed/value_type.yaml

@@ -0,0 +1,31 @@
+$schema: ../../../../../../json_schemas/test_story.schema.yaml
+
+description: This story has an error trying to reuse a variable of a different type.
+
+prologues:
+  - method: GET
+    id: root
+    path: /
+    output:
+      boolean: payload.version.build_snapshot
+epilogues:
+  - method: DELETE
+    path: /movies
+    status: [200, 404]
+chapters:
+  - synopsis: This chapter requires a boolean in the parameters.
+    method: GET
+    path: /_cat/health
+    parameters:
+      format: json
+      v: ${root.boolean}
+  - synopsis: This chapter requires a boolean in the body.
+    method: PUT
+    path: /{index}
+    parameters:
+      index: movies
+    request:
+      payload:
+        settings:
+          shard:
+            check_on_startup: ${root.boolean}

tools/tests/tester/integ/StoryEvaluator.test.ts

@@ -22,8 +22,14 @@ afterEach(() => {
 })
 
 test('passed', async () => {
-  const actual = await load_actual_evaluation(story_evaluator, 'passed')
-  const expected = load_expected_evaluation('passed')
+  const actual = await load_actual_evaluation(story_evaluator, 'passed/passed')
+  const expected = load_expected_evaluation('passed/passed')
+  expect(actual).toEqual(expected)
+})
+
+test('value_type.yaml', async () => {
+  const actual = await load_actual_evaluation(story_evaluator, 'passed/value_type')
+  const expected = load_expected_evaluation('passed/value_type')
   expect(actual).toEqual(expected)
 })
 
@@ -65,7 +71,7 @@ test('skipped/semver', async () => {
 
 test('with an unexpected error deserializing data', async () => {
   opensearch_http_client.request = jest.fn().mockRejectedValue(new Error('This was unexpected.'))
-  const actual = await load_actual_evaluation(story_evaluator, 'passed')
+  const actual = await load_actual_evaluation(story_evaluator, 'passed/passed')
   expect(actual.result).toEqual(Result.ERROR)
   expect(actual.chapters && actual.chapters[0]).toEqual({
     title: "This PUT /{index} chapter should pass.",

tools/tests/tester/integ/TestRunner.test.ts

@@ -29,19 +29,20 @@ test('stories folder', async () => {
   }
 
   const expected_evaluations = [
-    'passed',
     'error/chapter_error',
     'error/output_error',
     'error/prologue_error',
     'failed/invalid_data',
     'failed/not_found',
+    'passed/passed',
+    'passed/value_type',
     'skipped/semver',
     'skipped/distributions/chapters',
     'skipped/distributions/excluded',
     'skipped/distributions/included'
   ].map((fixture) => { return load_expected_evaluation(fixture, true) })
 
-  expect(actual_evaluations).toStrictEqual(expected_evaluations)
+  expect(actual_evaluations).toEqual(expected_evaluations)
 })
 
 describe('story_files', () => {