Skip to content

Commit

Permalink
Added support for skipping tests using semver range.
Browse files Browse the repository at this point in the history
Signed-off-by: dblock <[email protected]>
  • Loading branch information
dblock committed Jul 11, 2024
1 parent 8b26266 commit d5825c3
Show file tree
Hide file tree
Showing 24 changed files with 159 additions and 28 deletions.
12 changes: 8 additions & 4 deletions .github/workflows/test-spec.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,18 @@ jobs:
strategy:
matrix:
entry:
- {version: 2.15.0, hub: 'opensearchproject'}
- {version: 2.0.0, admin_password: 'admin'}
- {version: 2.15.0}
- {version: 2.16.0, hub: 'opensearchstaging'}
name: test-opensearch-spec (version=${{ matrix.entry.version }}, hub=${{ matrix.entry.hub }})

name: test-opensearch-spec (version=${{ matrix.entry.version }}, hub=${{ matrix.entry.hub || 'opensearchproject' }})
runs-on: ubuntu-latest

env:
OPENSEARCH_DOCKER_HUB_PROJECT: ${{ matrix.entry.hub }}
OPENSEARCH_DOCKER_HUB_PROJECT: ${{ matrix.entry.hub || 'opensearchproject' }}
OPENSEARCH_VERSION: ${{ matrix.entry.version }}
OPENSEARCH_PASSWORD: myStrongPassword123!
OPENSEARCH_PASSWORD: ${{ matrix.entry.admin_password || 'myStrongPassword123!' }}

steps:
- name: Checkout Repo
uses: actions/checkout@v4
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/test-tools-integ.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ jobs:
test:
runs-on: ubuntu-latest
env:
OPENSEARCH_VERSION: 2.12.0
OPENSEARCH_VERSION: 2.15.0
OPENSEARCH_PASSWORD: myStrongPassword123!
OPENSEARCH_URL: https://localhost:9200
steps:
Expand All @@ -35,7 +35,7 @@ jobs:
working-directory: .github/opensearch-cluster
run: |
docker-compose up -d
sleep 30
sleep 15
- name: Setup Node.js
uses: actions/setup-node@v3
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Added `remote_store` to `TranslogStats` ([#395](https://github.com/opensearch-project/opensearch-api-specification/pull/395))
- Added `file` to `/_cache/clear` and `/{index}/_cache/clear` ([#396](https://github.com/opensearch-project/opensearch-api-specification/pull/396))
- Added a workflow to run tests against the next version of OpenSearch ([#409](https://github.com/opensearch-project/opensearch-api-specification/pull/409))
- Added support for skipping tests using semver range ([#410](https://github.com/opensearch-project/opensearch-api-specification/pull/410))

### Changed

Expand Down
31 changes: 25 additions & 6 deletions SPECIFICATION_TESTING.md
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
<!-- TOC -->
* [Specification Testing](#specification-testing)
* [Running Spec Tests Locally](#running-spec-tests-locally)
* [Writing Spec Tests](#writing-spec-tests)
* [Simple Test Story](#simple-test-story)
* [Using Output from Previous Chapters](#using-output-from-previous-chapters)
- [Specification Testing](#specification-testing)
- [Running Spec Tests Locally](#running-spec-tests-locally)
- [Writing Spec Tests](#writing-spec-tests)
- [Simple Test Story](#simple-test-story)
- [Using Output from Previous Chapters](#using-output-from-previous-chapters)
- [Managing Versions](#managing-versions)
<!-- TOC -->

# Specification Testing
Expand Down Expand Up @@ -133,4 +134,22 @@ Consider the following chapters in [ml/model_groups](tests/ml/model_groups.yaml)
response:
status: 200
```
As you can see, the `output` field in the first chapter saves the `model_group_id` from the response body. This value is then used in the subsequent chapters to query and delete the model group.
As you can see, the `output` field in the first chapter saves the `model_group_id` from the response body. This value is then used in the subsequent chapters to query and delete the model group.
### Managing Versions
It's common to add a feature to the next version of OpenSearch. When adding a new API in the spec, make sure to specify `x-version-added`, `x-version-deprecated` or `x-version-removed`. Finally, specify a semver range in your tests as follows.
```yaml
- synopsis: Search with `phase_took` added in OpenSearch 2.12.
version: '>= 2.12.0'
path: /{index}/_search
parameters:
index: movies
cancel_after_time_interval: 10s
method: POST
response:
status: 200
```
The [integration test workflow](.github/workflows/test-spec.yml) runs a matrix of OpenSearch versions, including the next version. Please check whether the workflow needs an update when adding version-specific tests.
2 changes: 2 additions & 0 deletions json_schemas/_info.schema.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ properties:
- name
version:
type: string
x-api-version:
type: string
required:
- title
- version
7 changes: 7 additions & 0 deletions json_schemas/test_story.schema.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ definitions:
$ref: '#/definitions/RequestBody'
output:
$ref: '#/definitions/Output'
version:
$ref: '#/definitions/Version'
required: [path, method]

Output:
Expand All @@ -90,6 +92,11 @@ definitions:
additionalProperties:
type: string

Version:
description: |
The semver range to execute the chapter against.
type: string

RequestBody:
type: object
properties:
Expand Down
1 change: 1 addition & 0 deletions spec/_info.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ $schema: ./json_schemas/_info.schema.yaml

title: OpenSearch API Specification
version: 1.0.0
x-api-version: 2.15.0
1 change: 1 addition & 0 deletions spec/schemas/_core.search.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -852,6 +852,7 @@ components:
_shards:
$ref: '_common.yaml#/components/schemas/ShardStatistics'
phase_took:
x-version-added: '2.12.0'
$ref: '_common.yaml#/components/schemas/PhaseTook'
hits:
$ref: '#/components/schemas/HitsMetadata'
Expand Down
10 changes: 9 additions & 1 deletion tests/_core/search.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,19 @@ chapters:
director: Bennett Miller
title: Moneyball
year: 2011
- synopsis: Search with parameters.
- synopsis: Search with cancel_after_time_interval.
path: /{index}/_search
parameters:
index: movies
cancel_after_time_interval: 10s
method: POST
response:
status: 200
- synopsis: Search with phase_took.
version: '>= 2.12.0'
path: /{index}/_search
parameters:
index: movies
phase_took: true
method: POST
response:
Expand Down
2 changes: 1 addition & 1 deletion tools/src/OpenSearchHttpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ export class OpenSearchHttpClient {
}
} catch (e) {
if (axios.isAxiosError(e)) {
if (e.code === 'UNABLE_TO_VERIFY_LEAF_SIGNATURE') {
if (e.response?.status == 401 || e.code === 'UNABLE_TO_VERIFY_LEAF_SIGNATURE') {
throw e
}
}
Expand Down
4 changes: 4 additions & 0 deletions tools/src/tester/MergedOpenApiSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ export default class MergedOpenApiSpec {
return this._spec
}

api_version(): string {
return (this.spec().info as any)['x-api-version']
}

private inject_additional_properties(ctx: SpecificationContext, spec: OpenAPIV3.Document): void {
const visitor = new SchemaVisitor((_ctx, schema: any) => {
if (schema.required !== undefined && schema.properties !== undefined && schema.additionalProperties === undefined) {
Expand Down
10 changes: 7 additions & 3 deletions tools/src/tester/StoryEvaluator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { overall_result } from './helpers'
import { StoryOutputs } from './StoryOutputs'
import SupplementalChapterEvaluator from './SupplementalChapterEvaluator'
import { ChapterOutput } from './ChapterOutput'
import * as semver from 'semver'

export default class StoryEvaluator {
private readonly _chapter_evaluator: ChapterEvaluator
Expand All @@ -24,14 +25,14 @@ export default class StoryEvaluator {
this._supplemental_chapter_evaluator = supplemental_chapter_evaluator
}

async evaluate({ story, display_path, full_path }: StoryFile, dry_run: boolean = false): Promise<StoryEvaluation> {
async evaluate({ story, display_path, full_path }: StoryFile, version: string, dry_run: boolean = false): Promise<StoryEvaluation> {
const variables_error = StoryEvaluator.check_story_variables(story, display_path, full_path)
if (variables_error !== undefined) {
return variables_error
}
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, dry_run, story_outputs)
const chapters = await this.#evaluate_chapters(story.chapters, prologue_errors, version, dry_run, story_outputs)
const { evaluations: epilogues } = await this.#evaluate_supplemental_chapters(story.epilogues ?? [], dry_run, story_outputs)
return {
display_path,
Expand All @@ -44,12 +45,15 @@ export default class StoryEvaluator {
}
}

async #evaluate_chapters(chapters: Chapter[], has_errors: boolean, dry_run: boolean, story_outputs: StoryOutputs): Promise<ChapterEvaluation[]> {
async #evaluate_chapters(chapters: Chapter[], has_errors: boolean, version: string, dry_run: boolean, story_outputs: StoryOutputs): 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)) {
const title = chapter.synopsis || `${chapter.method} ${chapter.path}`
evaluations.push({ title, overall: { result: Result.SKIPPED, message: `Skipped because ${version} does not satisfy ${chapter.version}.`, error: undefined } })
} else {
const evaluation = await this._chapter_evaluator.evaluate(chapter, has_errors, story_outputs)
has_errors = has_errors || evaluation.overall.result === Result.ERROR
Expand Down
5 changes: 3 additions & 2 deletions tools/src/tester/TestRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,19 @@ export default class TestRunner {
this._result_logger = result_logger
}

async run (story_path: string, dry_run: boolean = false): Promise<{ evaluations: StoryEvaluation[], failed: boolean }> {
async run (story_path: string, version: string = '2.15.0', dry_run: boolean = false): Promise<{ evaluations: StoryEvaluation[], failed: boolean }> {
let failed = false
const story_files = this.#sort_story_files(this.#collect_story_files(resolve(story_path), '', ''))
const evaluations: StoryEvaluation[] = []

if (!dry_run) {
const info = await this._http_client.wait_until_available()
console.log(`OpenSearch ${ansi.green(info.version.number)}\n`)
version = info.version.number
}

for (const story_file of story_files) {
const evaluation = this._story_validator.validate(story_file) ?? await this._story_evaluator.evaluate(story_file, dry_run)
const evaluation = this._story_validator.validate(story_file) ?? await this._story_evaluator.evaluate(story_file, version, dry_run)
evaluations.push(evaluation)
this._result_logger.log(evaluation)
if ([Result.ERROR, Result.FAILED].includes(evaluation.result)) failed = true
Expand Down
3 changes: 2 additions & 1 deletion tools/src/tester/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@
import { type Evaluation, Result } from './types/eval.types'

export function overall_result(evaluations: Evaluation[]): Result {
if (evaluations.length == 0) return Result.PASSED
if (evaluations.some(e => e.result === Result.ERROR)) return Result.ERROR
if (evaluations.some(e => e.result === Result.FAILED)) return Result.FAILED
if (evaluations.some(e => e.result === Result.SKIPPED)) return Result.SKIPPED
if (evaluations.every(e => e.result === Result.SKIPPED)) return Result.SKIPPED
return Result.PASSED
}
6 changes: 3 additions & 3 deletions tools/src/tester/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,17 +50,17 @@ const command = new Command()
const opts = command.opts()
const logger = new Logger(opts.verbose ? LogLevel.info : LogLevel.warn)

const spec = (new MergedOpenApiSpec(opts.specPath, new Logger(LogLevel.error))).spec()
const spec = new MergedOpenApiSpec(opts.specPath, new Logger(LogLevel.error))
const http_client = new OpenSearchHttpClient(get_opensearch_opts_from_cli({ opensearchResponseType: 'arraybuffer', ...opts }))
const chapter_reader = new ChapterReader(http_client, logger)
const chapter_evaluator = new ChapterEvaluator(new OperationLocator(spec), chapter_reader, new SchemaValidator(spec, logger), logger)
const chapter_evaluator = new ChapterEvaluator(new OperationLocator(spec.spec()), chapter_reader, new SchemaValidator(spec.spec(), logger), logger)
const supplemental_chapter_evaluator = new SupplementalChapterEvaluator(chapter_reader)
const story_validator = new StoryValidator()
const story_evaluator = new StoryEvaluator(chapter_evaluator, supplemental_chapter_evaluator)
const result_logger = new ConsoleResultLogger(opts.tabWidth, opts.verbose)
const runner = new TestRunner(http_client, story_validator, story_evaluator, result_logger)

runner.run(opts.testsPath, opts.dryRun)
runner.run(opts.testsPath, spec.api_version(), opts.dryRun)
.then(
({ failed }) => {
if (failed) process.exit(1)
Expand Down
9 changes: 9 additions & 0 deletions tools/src/tester/types/story.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@ export type Parameter = (string | number | boolean)[] | string | number | boolea
* via the `definition` "Payload".
*/
export type Payload = {} | any[] | string | number | boolean;
/**
* The semver range to execute the chapter against.
*
*
* This interface was referenced by `Story`'s JSON-Schema
* via the `definition` "Version".
*/
export type Version = string;
/**
* This interface was referenced by `Story`'s JSON-Schema
* via the `definition` "Chapter".
Expand Down Expand Up @@ -79,6 +87,7 @@ export interface ChapterRequest {
};
request_body?: RequestBody;
output?: Output;
version?: Version;
}
/**
* This interface was referenced by `Story`'s JSON-Schema
Expand Down
4 changes: 4 additions & 0 deletions tools/tests/tester/MergedOpenApiSpec.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ describe('additionalProperties', () => {
const spec = new MergedOpenApiSpec('tools/tests/tester/fixtures/specs/complete', new Logger())
const responses: any = spec.spec().components?.responses

test('has an api version', () => {
expect(spec.api_version()).toEqual('1.2.3')
})

test('is added with required fields', () => {
const schema = responses['info@200'].content['application/json'].schema
expect(schema.additionalProperties).toEqual({ not: true, errorMessage: 'property is not defined in the spec' })
Expand Down
36 changes: 36 additions & 0 deletions tools/tests/tester/fixtures/evals/passed.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,42 @@ chapters:
result: PASSED
payload_schema:
result: PASSED
- title: This GET /_cat/health should run (default).
overall:
result: PASSED
request:
parameters:
format:
result: PASSED
request_body:
result: PASSED
response:
status:
result: PASSED
payload_body:
result: PASSED
payload_schema:
result: PASSED
- title: This GET /_cat/health should run (~> 2.x).
overall:
result: PASSED
request:
parameters:
format:
result: PASSED
request_body:
result: PASSED
response:
status:
result: PASSED
payload_body:
result: PASSED
payload_schema:
result: PASSED
- title: This GET /_cat/health should be skipped (> 2.999.0).
overall:
result: SKIPPED
message: Skipped because 2.15.0 does not satisfy >= 2.999.0.
epilogues:
- title: DELETE /books
overall:
Expand Down
3 changes: 2 additions & 1 deletion tools/tests/tester/fixtures/specs/complete/_info.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@ $schema: should-be-ignored

title: OpenSearch API
description: OpenSearch API
version: 1.0.0
version: 1.0.0
x-api-version: 1.2.3
17 changes: 17 additions & 0 deletions tools/tests/tester/fixtures/stories/passed.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,20 @@ chapters:
content_type: application/smile
payload:
- node.total: '1'
- synopsis: This GET /_cat/health should run (default).
method: GET
path: /_cat/health
parameters:
format: json
- synopsis: This GET /_cat/health should run (~> 2.x).
version: '~> 2.x'
method: GET
path: /_cat/health
parameters:
format: json
- synopsis: This GET /_cat/health should be skipped (> 2.999.0).
version: '>= 2.999.0'
method: GET
path: /_cat/health
parameters:
format: json
3 changes: 2 additions & 1 deletion tools/tests/tester/helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ describe('helpers', () => {
test('overall_result', () => {
expect(overall_result(e(Result.PASSED, Result.SKIPPED, Result.FAILED, Result.ERROR))).toBe(Result.ERROR)
expect(overall_result(e(Result.PASSED, Result.SKIPPED, Result.FAILED))).toBe(Result.FAILED)
expect(overall_result(e(Result.PASSED, Result.SKIPPED))).toBe(Result.SKIPPED)
expect(overall_result(e(Result.PASSED, Result.SKIPPED))).toBe(Result.PASSED)
expect(overall_result(e(Result.SKIPPED, Result.SKIPPED))).toBe(Result.SKIPPED)
expect(overall_result(e(Result.PASSED))).toBe(Result.PASSED)
expect(overall_result(e())).toBe(Result.PASSED)
})
Expand Down
2 changes: 1 addition & 1 deletion tools/tests/tester/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,5 +138,5 @@ export async function load_actual_evaluation (evaluator: StoryEvaluator, name: s
full_path,
display_path: `${name}.yaml`,
story: read_yaml(full_path)
}))
}, '1.2.3'))
}
Loading

0 comments on commit d5825c3

Please sign in to comment.