From 89f31e1b1c9478dc944142574e711b989219c409 Mon Sep 17 00:00:00 2001 From: Theo Truong Date: Mon, 24 Jun 2024 14:42:47 -0600 Subject: [PATCH] Added SPEC_TESTING.md - Reduced noise when running spec tests - `--verbose` now provides better context for non-2XX responses. - Moved `linter/utils` to `_utils` because it's also being used by the tester tool (To avoid misleading stacktrace when an error appears in utils) Signed-off-by: Theo Truong --- CHANGELOG.md | 4 + DEVELOPER_GUIDE.md | 166 ++++-------------- SPEC_TESTING.md | 136 ++++++++++++++ .../utils => _utils}/SpecificationVisitor.ts | 0 tools/src/{linter/utils => _utils}/index.ts | 0 .../src/linter/InlineObjectSchemaValidator.ts | 4 +- tools/src/tester/ChapterReader.ts | 5 +- tools/src/tester/MergedOpenApiSpec.ts | 4 +- tools/src/tester/test.ts | 11 +- tools/tests/linter/NamespacesFolder.test.ts | 2 +- 10 files changed, 193 insertions(+), 139 deletions(-) create mode 100644 SPEC_TESTING.md rename tools/src/{linter/utils => _utils}/SpecificationVisitor.ts (100%) rename tools/src/{linter/utils => _utils}/index.ts (100%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d0d14594..a26cbe8a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Added tests for response payload ([#347](https://github.com/opensearch-project/opensearch-api-specification/pull/347)) - Added `cancel_after_time_interval` and `phase_took` in `_search` ([#353](https://github.com/opensearch-project/opensearch-api-specification/pull/353)) - Added support for testing `application/x-ndjson` payloads ([#355](https://github.com/opensearch-project/opensearch-api-specification/pull/355)) +- Added SPEC_TESTING.md [#359](https://github.com/opensearch-project/opensearch-api-specification/pull/359) ### Changed @@ -33,6 +34,9 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Refactored spec tester internals to improve reusability ([#302](https://github.com/opensearch-project/opensearch-api-specification/pull/302)) - Renamed `main` release tag to `main-latest` ([#321](https://github.com/opensearch-project/opensearch-api-specification/pull/321)) - Replaced usages of `Opensearch` with `OpenSearch` ([#335](https://github.com/opensearch-project/opensearch-api-specification/pull/335)) +- Prevented merger tool from printing warnings when used by tester tool [#359](https://github.com/opensearch-project/opensearch-api-specification/pull/359) +- Replaced the deprecated fs.rmdirSync with fs.rmSync [#359](https://github.com/opensearch-project/opensearch-api-specification/pull/359) +- Tester tool now provides better context for non-2XX responses when --verbose is used [#359](https://github.com/opensearch-project/opensearch-api-specification/pull/359) ### Deprecated diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index 64f4f8358..846f98689 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -1,42 +1,42 @@ -- [Developer Guide](#developer-guide) - - [Getting Started](#getting-started) - - [Specification](#specification) - - [File Structure](#file-structure) - - [Grouping Operations](#grouping-operations) - - [Grouping Schemas](#grouping-schemas) - - [Superseded Operations](#superseded-operations) - - [Global Parameters](#global-parameters) - - [OpenAPI Extensions](#openapi-extensions) - - [Writing Spec Tests](#writing-spec-tests) - - [Test Stories](#test-stories) - - [Organizing Tests](#organizing-tests) - - [Running Spec Tests Locally](#running-spec-tests-locally) - - [Tools](#tools) - - [Setup](#setup) - - [Spec Merger](#spec-merger) - - [Arguments](#arguments) - - [Example](#example) - - [Spec Linter](#spec-linter) - - [Arguments](#arguments-1) - - [Example](#example-1) - - [Dump Cluster Spec](#dump-cluster-spec) - - [Arguments](#arguments-2) - - [Example](#example-2) - - [Coverage](#coverage) - - [Arguments](#arguments-3) - - [Example](#example-3) - - [Testing](#testing) - - [Tests](#tests) - - [Lints](#lints) - - [Workflows](#workflows) - - [Analyze PR Changes](#analyze-pr-changes) - - [Build](#build) - - [Deploy GitHub Pages](#deploy-github-pages) - - [Comment on PR](#comment-on-pr) - - [Test Tools (Unit)](#test-tools-unit) - - [Test Tools (Integration)](#test-tools-integration) - - [Validate Spec](#validate-spec) +* [Developer Guide](#developer-guide) + * [Getting Started](#getting-started) + * [Specification](#specification) + * [File Structure](#file-structure) + * [Grouping Operations](#grouping-operations) + * [Grouping Schemas](#grouping-schemas) + * [Superseded Operations](#superseded-operations) + * [Global Parameters](#global-parameters) + * [OpenAPI Extensions](#openapi-extensions) + * [Writing Spec Tests](#writing-spec-tests) + * [Test Stories](#test-stories) + * [Organizing Tests](#organizing-tests) + * [Running Spec Tests Locally](#running-spec-tests-locally) + * [Tools](#tools) + * [Setup](#setup) + * [Spec Merger](#spec-merger) + * [Arguments](#arguments) + * [Example](#example) + * [Spec Linter](#spec-linter) + * [Arguments](#arguments-1) + * [Example](#example-1) + * [Dump Cluster Spec](#dump-cluster-spec) + * [Arguments](#arguments-2) + * [Example](#example-2) + * [Coverage](#coverage) + * [Arguments](#arguments-3) + * [Example](#example-3) + * [Testing](#testing) + * [Tests](#tests) + * [Lints](#lints) + * [Workflows](#workflows) + * [Analyze PR Changes](#analyze-pr-changes) + * [Build](#build) + * [Deploy GitHub Pages](#deploy-github-pages) + * [Comment on PR](#comment-on-pr) + * [Test Tools (Unit)](#test-tools--unit-) + * [Test Tools (Integration)](#test-tools--integration-) + * [Validate Spec](#validate-spec) # Developer Guide @@ -152,95 +152,7 @@ This repository includes several OpenAPI Specification Extensions to fill in any ## Writing Spec Tests -To assure the correctness of the spec, you must add tests for the spec in the [tests/](tests) directory. - -### Test Stories - -Each yaml file in the tests directory represents a test story that tests a collection of related operations. - -A test story has 3 main components: -- prologues: These are the operations that are executed before the test story is run. They are used to set up the environment for the test story. -- chapters: These are the operations that are being tested. -- epilogues: These are the operations that are executed after the test story is run. They are used to clean up the environment after the test story. - -Below is the simplified version of the test story that tests the [index operations](tests/indices/index.yaml): -```yaml -$schema: ../json_schemas/test_story.schema.yaml # The schema of the test story. Include this line so that your editor can validate the test story on the fly. - -description: This story tests all endpoints relevant the lifecycle of an index, from creation to deletion. - -prologues: [] # No prologues are needed for this story. - -epilogues: # Clean up the environment by assuring that the `books` index is deleted afterward. - - path: /books - method: DELETE - status: [200, 404] # The index may not exist, so we accept 404 as a valid response. Default to [200, 201] if not specified. - -chapters: - - synopsis: Create an index named `books` with mappings and settings. - path: /{index} # The test will fail if "PUT /{index}" operation is not found in the spec. - method: PUT - parameters: # All parameters are validated against their schemas in the spec - index: books - request_body: # The request body is validated against the schema of the requestBody in the spec - payload: - mappings: - properties: - name: - type: keyword - age: - type: integer - settings: - number_of_shards: 5 - number_of_replicas: 2 - response: # The response body is validated against the schema of the corresponding response in the spec - status: 200 # This is the expected status code of the response. Any other status code will fail the test. - - - synopsis: Retrieve the mappings and settings of the `books` index. - path: /{index} - method: GET - parameters: - index: books - flat_settings: true - - - synopsis: Delete the `books` index. - path: /{index} - method: DELETE - parameters: - index: books -``` - -Check the [test_story JSON Schema](json_schemas/test_story.schema.yaml) for the complete structure of a test story. - -### Organizing Tests - -Tests are organized in folders that match [namespaces](spec/namespaces). For example, tests for APIs defined in [spec/namespaces/indices.yaml](spec/namespaces/indices.yaml) can be found in [tests/indices/index.yaml](tests/indices/index.yaml) (for `/{index}`), and [tests/indices/_doc.yaml](tests/indices/_doc.yaml) (for `/{index}/_doc`). - -### Running Spec Tests Locally - -Set up an OpenSearch cluster with Docker using the default `OPENSEARCH_PASSWORD` (Recommended): -```bash -cd .github/opensearch-cluster -docker-compose up -d -``` - -Run the tests (use `--opensearch-insecure` for a local cluster running in Docker that does not have a valid SSL certificate): -```bash -npm run test:spec -- --opensearch-insecure -``` - -Run a specific test story: -```bash -npm run test:spec -- --opensearch-insecure --tests tests/_core/info.yaml -``` - -If you opt to use a different password, you can set the `OPENSEARCH_PASSWORD` environment variable to the desired password before running `docker-compose up` and every time you run the tests: -```bash -export OPENSEARCH_PASSWORD='yourOwnPassword@2021' -``` - -Check the [test_story JSON Schema](json_schemas/test_story.schema.yaml) for the complete structure of a test story. - +To assure the correctness of the spec, you must add tests for the spec, when making changes. Check [SPEC_TESTING.md](SPEC_TESTING.md) for more information. ## Tools diff --git a/SPEC_TESTING.md b/SPEC_TESTING.md new file mode 100644 index 000000000..873a70f98 --- /dev/null +++ b/SPEC_TESTING.md @@ -0,0 +1,136 @@ + +* [SPEC TESTING](#spec-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) + + +# SPEC TESTING + +We devise our own test framework to test the spec against an OpenSearch cluster. We're still adding more features to the framework as the needs arise, and this document will be updated accordingly. This test framework has also been integrated into the repo's CI/CD pipeline. Checkout the [test-spec](.github/workflows/test-spec.yml) workflow for more details. + +## Running Spec Tests Locally + +Set up an OpenSearch cluster with Docker: + +(Replace `<>` with your desired password. If not provided, the default password inside the `docker-compose.yml` file will be used.) +```bash +export OPENSEARCH_PASSWORD=<> +cd .github/opensearch-cluster +docker-compose up -d +``` + +Run the tests (use `--opensearch-insecure` for a local cluster running in Docker that does not have a valid SSL certificate): +```bash +npm run test:spec -- --opensearch-insecure +``` + +Run a specific test story: +```bash +npm run test:spec -- --opensearch-insecure --tests tests/_core/info.yaml +``` + +Verbose output: +```bash +npm run test:spec -- --opensearch-insecure --verbose +``` + +Note: Remember to set the `OPENSEARCH_PASSWORD` environment variable everytime you start a new shell to run the tests. + +## Writing Spec Tests + +The spec tests reside in the [tests/](tests) directory. Tests are organized in folders that match [namespaces](spec/namespaces). For example, tests for APIs defined in [spec/namespaces/indices.yaml](spec/namespaces/indices.yaml) can be found in [tests/indices/index.yaml](tests/indices/index.yaml) (for `/{index}`), and [tests/indices/_doc.yaml](tests/indices/_doc.yaml) (for `/{index}/_doc`). + +Each yaml file in the tests directory represents a test story that tests a collection of related operations. + +A test story has 3 main components: +- prologues: These are the operations that are executed before the test story is run. They are used to set up the environment for the test story. +- chapters: These are the operations that are being tested. +- epilogues: These are the operations that are executed after the test story is run. They are used to clean up the environment after the test story. + +Check the [test_story JSON Schema](json_schemas/test_story.schema.yaml) for the complete structure of a test story. + +### Simple Test Story + +Below is the simplified version of the test story that tests the [index operations](tests/indices/index.yaml): +```yaml +$schema: ../json_schemas/test_story.schema.yaml # The schema of the test story. Include this line so that your editor can validate the test story on the fly. + +description: This story tests all endpoints relevant the lifecycle of an index, from creation to deletion. + +prologues: [] # No prologues are needed for this story. + +epilogues: # Clean up the environment by assuring that the `books` index is deleted afterward. + - path: /books + method: DELETE + status: [200, 404] # The index may not exist, so we accept 404 as a valid response. Default to [200, 201] if not specified. + +chapters: + - synopsis: Create an index named `books` with mappings and settings. + path: /{index} # The test will fail if "PUT /{index}" operation is not found in the spec. + method: PUT + parameters: # All parameters are validated against their schemas in the spec + index: books + request_body: # The request body is validated against the schema of the requestBody in the spec + payload: + mappings: + properties: + name: + type: keyword + age: + type: integer + settings: + number_of_shards: 5 + number_of_replicas: 2 + response: # The response body is validated against the schema of the corresponding response in the spec + status: 200 # This is the expected status code of the response. Any other status code will fail the test. + + - synopsis: Retrieve the mappings and settings of the `books` index. + path: /{index} + method: GET + parameters: + index: books + flat_settings: true + + - synopsis: Delete the `books` index. + path: /{index} + method: DELETE + parameters: + index: books +``` + +### Using Output from Previous Chapters + +Consider the following chapters in [ml/model_groups](tests/ml/model_groups.yaml) test story: +```yaml + - synopsis: Create model group. + id: create_model_group # Only needed if you want to refer to this chapter in another chapter. + path: /_plugins/_ml/model_groups/_register + method: POST + request_body: + payload: + name: "NLP_Group" + description: "Model group for NLP models" + response: + status: 200 + output: # Save the model group id for later use. + test_model_group_id: "payload.model_group_id" + - synopsis: Query model group. + path: /_plugins/_ml/model_groups/{model_group_id} + method: GET + parameters: + # Use the output from the `create_model_group` chapter. + model_group_id: ${create_model_group.test_model_group_id} + response: + status: 200 + - synopsis: Delete model group. + path: /_plugins/_ml/model_groups/{model_group_id} + method: DELETE + parameters: + # Use the output from the `create_model_group` chapter. + model_group_id: ${create_model_group.test_model_group_id} + 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 diff --git a/tools/src/linter/utils/SpecificationVisitor.ts b/tools/src/_utils/SpecificationVisitor.ts similarity index 100% rename from tools/src/linter/utils/SpecificationVisitor.ts rename to tools/src/_utils/SpecificationVisitor.ts diff --git a/tools/src/linter/utils/index.ts b/tools/src/_utils/index.ts similarity index 100% rename from tools/src/linter/utils/index.ts rename to tools/src/_utils/index.ts diff --git a/tools/src/linter/InlineObjectSchemaValidator.ts b/tools/src/linter/InlineObjectSchemaValidator.ts index 460832f6f..1f3e06134 100644 --- a/tools/src/linter/InlineObjectSchemaValidator.ts +++ b/tools/src/linter/InlineObjectSchemaValidator.ts @@ -10,8 +10,8 @@ import type NamespacesFolder from './components/NamespacesFolder' import type SchemasFolder from './components/SchemasFolder' import { type ValidationError } from 'types' -import { SchemaVisitor } from './utils/SpecificationVisitor' -import { is_ref, type MaybeRef, SpecificationContext } from './utils' +import { SchemaVisitor } from '../_utils/SpecificationVisitor' +import { is_ref, type MaybeRef, SpecificationContext } from '../_utils' import { type OpenAPIV3 } from 'openapi-types' export default class InlineObjectSchemaValidator { diff --git a/tools/src/tester/ChapterReader.ts b/tools/src/tester/ChapterReader.ts index fd9dd6acf..e34edb170 100644 --- a/tools/src/tester/ChapterReader.ts +++ b/tools/src/tester/ChapterReader.ts @@ -48,12 +48,13 @@ export default class ChapterReader { this.logger.info(`<= ERROR: ${e}`) throw e } - this.logger.info(`<= ${e.response.status} (${e.response.headers['content-type']}) | ${to_json(e.response.data)}`) response.status = e.response.status response.content_type = e.response.headers['content-type'].split(';')[0] response.payload = e.response.data?.error - response.message = e.response.data?.error?.reason + response.message = e.response.data?.error?.reason ?? e.response.statusText response.error = e + + this.logger.info(`<= ${response.status} (${response.content_type}) | ${to_json(response.payload ?? response.message)}`) }) return response as ActualResponse } diff --git a/tools/src/tester/MergedOpenApiSpec.ts b/tools/src/tester/MergedOpenApiSpec.ts index 6ae5dd410..e00ee11cb 100644 --- a/tools/src/tester/MergedOpenApiSpec.ts +++ b/tools/src/tester/MergedOpenApiSpec.ts @@ -9,8 +9,8 @@ import { type OpenAPIV3 } from 'openapi-types' import { Logger } from '../Logger' -import { SpecificationContext } from '../linter/utils'; -import { SchemaVisitor } from '../linter/utils/SpecificationVisitor'; +import { SpecificationContext } from '../_utils'; +import { SchemaVisitor } from '../_utils/SpecificationVisitor'; import OpenApiMerger from '../merger/OpenApiMerger'; // An augmented spec with additionalProperties: false. diff --git a/tools/src/tester/test.ts b/tools/src/tester/test.ts index db7a23664..8629e6b4b 100644 --- a/tools/src/tester/test.ts +++ b/tools/src/tester/test.ts @@ -7,22 +7,23 @@ * compatible open source license. */ -import { LogLevel, Logger } from '../Logger' +import {Logger, LogLevel} from '../Logger' import TestRunner from './TestRunner' -import { Command, Option } from '@commander-js/extra-typings' +import {Command, Option} from '@commander-js/extra-typings' import { get_opensearch_opts_from_cli, OPENSEARCH_INSECURE_OPTION, OPENSEARCH_PASSWORD_OPTION, OPENSEARCH_URL_OPTION, - OPENSEARCH_USERNAME_OPTION, OpenSearchHttpClient + OPENSEARCH_USERNAME_OPTION, + OpenSearchHttpClient } from '../OpenSearchHttpClient' import ChapterReader from './ChapterReader' import ChapterEvaluator from './ChapterEvaluator' import OperationLocator from './OperationLocator' import SchemaValidator from './SchemaValidator' import StoryEvaluator from './StoryEvaluator' -import { ConsoleResultLogger } from './ResultLogger' +import {ConsoleResultLogger} from './ResultLogger' import * as process from 'node:process' import SupplementalChapterEvaluator from './SupplementalChapterEvaluator' import MergedOpenApiSpec from './MergedOpenApiSpec' @@ -48,7 +49,7 @@ const command = new Command() const opts = command.opts() const logger = new Logger(opts.verbose ? LogLevel.info : LogLevel.warn) -const spec = (new MergedOpenApiSpec(opts.specPath, logger)).spec() +const spec = (new MergedOpenApiSpec(opts.specPath, new Logger(LogLevel.error))).spec() const http_client = new OpenSearchHttpClient(get_opensearch_opts_from_cli(opts)) const chapter_reader = new ChapterReader(http_client, logger) const chapter_evaluator = new ChapterEvaluator(new OperationLocator(spec), chapter_reader, new SchemaValidator(spec, logger)) diff --git a/tools/tests/linter/NamespacesFolder.test.ts b/tools/tests/linter/NamespacesFolder.test.ts index 380c67194..35b48babc 100644 --- a/tools/tests/linter/NamespacesFolder.test.ts +++ b/tools/tests/linter/NamespacesFolder.test.ts @@ -69,7 +69,7 @@ describe('validate()', () => { }) afterAll(() => { - fs.rmdirSync(invalid_yaml_path, { recursive: true }) + fs.rmSync(invalid_yaml_path, { recursive: true }) }) test('fails unable to parse YAML', () => {