diff --git a/.github/workflows/test-spec.yml b/.github/workflows/test-spec.yml index 528dcb697..4ba12acb4 100644 --- a/.github/workflows/test-spec.yml +++ b/.github/workflows/test-spec.yml @@ -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 diff --git a/.github/workflows/test-tools-integ.yml b/.github/workflows/test-tools-integ.yml index 2cb475239..807e1be0f 100644 --- a/.github/workflows/test-tools-integ.yml +++ b/.github/workflows/test-tools-integ.yml @@ -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: @@ -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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 2133140de..191d344d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/SPECIFICATION_TESTING.md b/SPECIFICATION_TESTING.md index b40283e4c..65386f57c 100644 --- a/SPECIFICATION_TESTING.md +++ b/SPECIFICATION_TESTING.md @@ -1,9 +1,10 @@ -* [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) # Specification Testing @@ -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. \ No newline at end of file +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. diff --git a/json_schemas/_info.schema.yaml b/json_schemas/_info.schema.yaml index 6b1ce5383..27cd9ae9d 100644 --- a/json_schemas/_info.schema.yaml +++ b/json_schemas/_info.schema.yaml @@ -38,6 +38,8 @@ properties: - name version: type: string + x-api-version: + type: string required: - title - version \ No newline at end of file diff --git a/json_schemas/test_story.schema.yaml b/json_schemas/test_story.schema.yaml index 3c3e28f53..cff3111fa 100644 --- a/json_schemas/test_story.schema.yaml +++ b/json_schemas/test_story.schema.yaml @@ -76,6 +76,8 @@ definitions: $ref: '#/definitions/RequestBody' output: $ref: '#/definitions/Output' + version: + $ref: '#/definitions/Version' required: [path, method] Output: @@ -90,6 +92,11 @@ definitions: additionalProperties: type: string + Version: + description: | + The semver range to execute the chapter against. + type: string + RequestBody: type: object properties: diff --git a/spec/_info.yaml b/spec/_info.yaml index 0b88a7355..2454ecb55 100644 --- a/spec/_info.yaml +++ b/spec/_info.yaml @@ -2,3 +2,4 @@ $schema: ./json_schemas/_info.schema.yaml title: OpenSearch API Specification version: 1.0.0 +x-api-version: 2.15.0 diff --git a/spec/schemas/_core.search.yaml b/spec/schemas/_core.search.yaml index 1011120e1..0bf65b1b8 100644 --- a/spec/schemas/_core.search.yaml +++ b/spec/schemas/_core.search.yaml @@ -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' diff --git a/tests/_core/search.yaml b/tests/_core/search.yaml index bb574de68..9a5190fe7 100644 --- a/tests/_core/search.yaml +++ b/tests/_core/search.yaml @@ -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: diff --git a/tools/src/OpenSearchHttpClient.ts b/tools/src/OpenSearchHttpClient.ts index 9d9a7f1a6..2652cd101 100644 --- a/tools/src/OpenSearchHttpClient.ts +++ b/tools/src/OpenSearchHttpClient.ts @@ -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 } } diff --git a/tools/src/tester/MergedOpenApiSpec.ts b/tools/src/tester/MergedOpenApiSpec.ts index e00ee11cb..612cd5b13 100644 --- a/tools/src/tester/MergedOpenApiSpec.ts +++ b/tools/src/tester/MergedOpenApiSpec.ts @@ -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) { diff --git a/tools/src/tester/StoryEvaluator.ts b/tools/src/tester/StoryEvaluator.ts index ecc6d3b6c..2a5998a2e 100644 --- a/tools/src/tester/StoryEvaluator.ts +++ b/tools/src/tester/StoryEvaluator.ts @@ -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 @@ -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 { + async evaluate({ story, display_path, full_path }: StoryFile, version: string, dry_run: boolean = false): Promise { 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, @@ -44,12 +45,15 @@ export default class StoryEvaluator { } } - async #evaluate_chapters(chapters: Chapter[], has_errors: boolean, dry_run: boolean, story_outputs: StoryOutputs): Promise { + async #evaluate_chapters(chapters: Chapter[], has_errors: boolean, version: string, dry_run: boolean, story_outputs: StoryOutputs): Promise { 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 diff --git a/tools/src/tester/TestRunner.ts b/tools/src/tester/TestRunner.ts index cfe286127..72f38d4f4 100644 --- a/tools/src/tester/TestRunner.ts +++ b/tools/src/tester/TestRunner.ts @@ -32,7 +32,7 @@ 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[] = [] @@ -40,10 +40,11 @@ export default class TestRunner { 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 diff --git a/tools/src/tester/helpers.ts b/tools/src/tester/helpers.ts index 684a57e27..e13aaeb23 100644 --- a/tools/src/tester/helpers.ts +++ b/tools/src/tester/helpers.ts @@ -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 } diff --git a/tools/src/tester/test.ts b/tools/src/tester/test.ts index 9a9ee3c8a..6fa7b5f68 100644 --- a/tools/src/tester/test.ts +++ b/tools/src/tester/test.ts @@ -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) diff --git a/tools/src/tester/types/story.types.ts b/tools/src/tester/types/story.types.ts index 54641cc70..7c5b1d0df 100644 --- a/tools/src/tester/types/story.types.ts +++ b/tools/src/tester/types/story.types.ts @@ -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". @@ -79,6 +87,7 @@ export interface ChapterRequest { }; request_body?: RequestBody; output?: Output; + version?: Version; } /** * This interface was referenced by `Story`'s JSON-Schema diff --git a/tools/tests/tester/MergedOpenApiSpec.test.ts b/tools/tests/tester/MergedOpenApiSpec.test.ts index c35699dc6..fd4218dc8 100644 --- a/tools/tests/tester/MergedOpenApiSpec.test.ts +++ b/tools/tests/tester/MergedOpenApiSpec.test.ts @@ -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' }) diff --git a/tools/tests/tester/fixtures/evals/passed.yaml b/tools/tests/tester/fixtures/evals/passed.yaml index 545c53249..3b4c9ca23 100644 --- a/tools/tests/tester/fixtures/evals/passed.yaml +++ b/tools/tests/tester/fixtures/evals/passed.yaml @@ -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: diff --git a/tools/tests/tester/fixtures/specs/complete/_info.yaml b/tools/tests/tester/fixtures/specs/complete/_info.yaml index 721aed8c9..acbf44f19 100644 --- a/tools/tests/tester/fixtures/specs/complete/_info.yaml +++ b/tools/tests/tester/fixtures/specs/complete/_info.yaml @@ -2,4 +2,5 @@ $schema: should-be-ignored title: OpenSearch API description: OpenSearch API -version: 1.0.0 \ No newline at end of file +version: 1.0.0 +x-api-version: 1.2.3 \ No newline at end of file diff --git a/tools/tests/tester/fixtures/stories/passed.yaml b/tools/tests/tester/fixtures/stories/passed.yaml index dffd3622e..ec187f219 100644 --- a/tools/tests/tester/fixtures/stories/passed.yaml +++ b/tools/tests/tester/fixtures/stories/passed.yaml @@ -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 diff --git a/tools/tests/tester/helpers.test.ts b/tools/tests/tester/helpers.test.ts index 3395e3087..5054dcdb7 100644 --- a/tools/tests/tester/helpers.test.ts +++ b/tools/tests/tester/helpers.test.ts @@ -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) }) diff --git a/tools/tests/tester/helpers.ts b/tools/tests/tester/helpers.ts index fb1217902..1c6259782 100644 --- a/tools/tests/tester/helpers.ts +++ b/tools/tests/tester/helpers.ts @@ -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')) } diff --git a/tools/tests/tester/integ/StoryEvaluator.test.ts b/tools/tests/tester/integ/StoryEvaluator.test.ts index 41d82ec64..ab70d3d46 100644 --- a/tools/tests/tester/integ/StoryEvaluator.test.ts +++ b/tools/tests/tester/integ/StoryEvaluator.test.ts @@ -9,7 +9,12 @@ import { construct_tester_components, load_actual_evaluation, load_expected_evaluation } from '../helpers' -const { story_evaluator } = construct_tester_components('tools/tests/tester/fixtures/specs/excerpt.yaml') +const { story_evaluator, opensearch_http_client } = construct_tester_components('tools/tests/tester/fixtures/specs/excerpt.yaml') + +beforeAll(async () => { + const info = await opensearch_http_client.wait_until_available() + expect(info.version).toBeDefined() +}) test('passed', async () => { const actual = await load_actual_evaluation(story_evaluator, 'passed') @@ -40,3 +45,4 @@ test('error/chapter_error', async () => { const expected = load_expected_evaluation('error/chapter_error') expect(actual).toEqual(expected) }) + diff --git a/tools/tests/tester/integ/TestRunner.test.ts b/tools/tests/tester/integ/TestRunner.test.ts index 87decadb4..250a1abfe 100644 --- a/tools/tests/tester/integ/TestRunner.test.ts +++ b/tools/tests/tester/integ/TestRunner.test.ts @@ -11,7 +11,11 @@ import { construct_tester_components, flatten_errors, load_expected_evaluation } import { type StoryEvaluation } from 'tester/types/eval.types' test('stories folder', async () => { - const { test_runner } = construct_tester_components('tools/tests/tester/fixtures/specs/excerpt.yaml') + const { test_runner, opensearch_http_client } = construct_tester_components('tools/tests/tester/fixtures/specs/excerpt.yaml') + + const info = await opensearch_http_client.wait_until_available() + expect(info.version).toBeDefined() + const result = await test_runner.run('tools/tests/tester/fixtures/stories') expect(result.failed).toBeTruthy()