From d14327c7bf146976ec4878ba90d198eed6abad9f Mon Sep 17 00:00:00 2001 From: dblock Date: Thu, 6 Jun 2024 13:15:39 -0400 Subject: [PATCH] Added command line tool tests for merger and linter. Signed-off-by: dblock --- DEVELOPER_GUIDE.md | 2 +- package.json | 1 + tools/src/linter/SchemasValidator.ts | 8 +++++--- tools/src/linter/SpecValidator.ts | 7 +++++-- tools/src/linter/lint.ts | 13 +++++++----- tools/src/merger/OpenApiMerger.ts | 4 ++-- tools/src/merger/SupersededOpsGenerator.ts | 6 +++--- tools/src/tester/start.ts | 5 +++-- tools/tests/linter/SchemasValidator.test.ts | 5 +++-- tools/tests/linter/SpecValidator.test.ts | 3 ++- tools/tests/linter/lint.test.ts | 22 +++++++++++++++++++++ tools/tests/merger/OpenApiMerger.test.ts | 4 ++-- tools/tests/merger/merge.test.ts | 22 +++++++++++++++++++++ tools/tests/tester/start.test.ts | 5 ++++- 14 files changed, 83 insertions(+), 24 deletions(-) create mode 100644 tools/tests/linter/lint.test.ts create mode 100644 tools/tests/merger/merge.test.ts diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index d8efe265a..c27576b2b 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -380,7 +380,7 @@ npm run test Specify the test path to run tests for one of the tools: ```bash -npm run test -- tools/tests/tester/ +npm run jest -- tools/tests/linter/lint.test.ts ``` The test suite contains unit tests and integration tests. Integration tests, such as [these](tools/tests/tester/integ/), require a local instance of OpenSearch and are placed into a folder named `integ`. Unit tests are run in parallel and integration tests are run sequentially using `--runInBand`. You can run unit tests with `npm run test:unit` separately from integration tests with `npm run test:integ`. diff --git a/package.json b/package.json index 032861043..8ce4a77ea 100644 --- a/package.json +++ b/package.json @@ -12,6 +12,7 @@ "lint--fix": "eslint . --fix", "merge": "ts-node tools/src/merger/merge.ts", "test": "npm run test:unit && npm run test:integ", + "jest": "jest", "test:unit": "jest --testMatch='**/*.test.ts' --testPathIgnorePatterns=/integ/", "test:integ": "jest --testMatch='**/integ/*.test.ts' --runInBand", "test:spec": "ts-node tools/src/tester/start.ts" diff --git a/tools/src/linter/SchemasValidator.ts b/tools/src/linter/SchemasValidator.ts index 2550d7f89..9390f09a1 100644 --- a/tools/src/linter/SchemasValidator.ts +++ b/tools/src/linter/SchemasValidator.ts @@ -11,7 +11,7 @@ import AJV from 'ajv' import addFormats from 'ajv-formats' import OpenApiMerger from '../merger/OpenApiMerger' import { type ValidationError } from '../types' -import { LogLevel } from '../Logger' +import { type Logger } from '../Logger' const IGNORED_ERROR_PREFIXES = [ 'can\'t resolve reference', // errors in referenced schemas will also cause reference errors @@ -26,11 +26,13 @@ const ADDITIONAL_KEYWORDS = [ ] export default class SchemasValidator { + logger: Logger root_folder: string spec: Record = {} ajv: AJV - constructor (root_folder: string) { + constructor (root_folder: string, logger: Logger) { + this.logger = logger this.root_folder = root_folder this.ajv = new AJV({ strict: true, discriminator: true }) addFormats(this.ajv) @@ -38,7 +40,7 @@ export default class SchemasValidator { } validate (): ValidationError[] { - this.spec = new OpenApiMerger(this.root_folder, LogLevel.error).merge().components as Record + this.spec = new OpenApiMerger(this.root_folder, this.logger).merge().components as Record const named_schemas_errors = this.validate_named_schemas() if (named_schemas_errors.length > 0) return named_schemas_errors return [ diff --git a/tools/src/linter/SpecValidator.ts b/tools/src/linter/SpecValidator.ts index f953b1efc..b954d3acc 100644 --- a/tools/src/linter/SpecValidator.ts +++ b/tools/src/linter/SpecValidator.ts @@ -15,8 +15,10 @@ import SupersededOperationsFile from './components/SupersededOperationsFile' import InfoFile from './components/InfoFile' import InlineObjectSchemaValidator from './InlineObjectSchemaValidator' import SchemasValidator from './SchemasValidator' +import { type Logger } from '../Logger' export default class SpecValidator { + logger: Logger superseded_ops_file: SupersededOperationsFile info_file: InfoFile namespaces_folder: NamespacesFolder @@ -25,12 +27,13 @@ export default class SpecValidator { schema_refs_validator: SchemaRefsValidator inline_object_schema_validator: InlineObjectSchemaValidator - constructor (root_folder: string) { + constructor (root_folder: string, logger: Logger) { + this.logger = logger this.superseded_ops_file = new SupersededOperationsFile(`${root_folder}/_superseded_operations.yaml`) this.info_file = new InfoFile(`${root_folder}/_info.yaml`) this.namespaces_folder = new NamespacesFolder(`${root_folder}/namespaces`) this.schemas_folder = new SchemasFolder(`${root_folder}/schemas`) - this.schemas_validator = new SchemasValidator(root_folder) + this.schemas_validator = new SchemasValidator(root_folder, logger) this.schema_refs_validator = new SchemaRefsValidator(this.namespaces_folder, this.schemas_folder) this.inline_object_schema_validator = new InlineObjectSchemaValidator(this.namespaces_folder, this.schemas_folder) } diff --git a/tools/src/linter/lint.ts b/tools/src/linter/lint.ts index f6ddcf5dd..af71a5084 100644 --- a/tools/src/linter/lint.ts +++ b/tools/src/linter/lint.ts @@ -10,24 +10,27 @@ import { Command, Option } from '@commander-js/extra-typings' import SpecValidator from './SpecValidator' import { resolve } from 'path' +import { Logger, LogLevel } from '../Logger' const command = new Command() .description('Validate the OpenSearch multi-file spec.') .addOption(new Option('-s, --source ', 'path to the root folder of the multi-file spec').default(resolve(__dirname, '../../../spec'))) + .addOption(new Option('--verbose', 'whether to print the full stack trace of errors').default(false)) .allowExcessArguments(false) .parse() const opts = command.opts() -console.log(`Validating ${opts.source} ...`) -const validator = new SpecValidator(opts.source) +const logger = new Logger(opts.verbose ? LogLevel.info : LogLevel.warn) +logger.log(`Validating ${opts.source} ...`) +const validator = new SpecValidator(opts.source, logger) const errors = validator.validate() if (errors.length === 0) { - console.log('No errors found.') + logger.log('No errors found.') process.exit(0) } else { - console.log('Errors found:\n') + logger.error('Errors found:\n') errors.forEach(e => { console.error(e) }) - console.log('\nTotal errors:', errors.length) + logger.error(`\nTotal errors:${errors.length}`) process.exit(1) } diff --git a/tools/src/merger/OpenApiMerger.ts b/tools/src/merger/OpenApiMerger.ts index 67af89e56..47decaeaa 100644 --- a/tools/src/merger/OpenApiMerger.ts +++ b/tools/src/merger/OpenApiMerger.ts @@ -13,7 +13,7 @@ import _ from 'lodash' import { read_yaml, write_yaml } from '../../helpers' import SupersededOpsGenerator from './SupersededOpsGenerator' import GlobalParamsGenerator from './GlobalParamsGenerator' -import { Logger, LogLevel } from '../Logger' +import { Logger } from '../Logger' // Create a single-file OpenAPI spec from multiple files for OpenAPI validation and programmatic consumption export default class OpenApiMerger { @@ -24,7 +24,7 @@ export default class OpenApiMerger { paths: Record> = {} // namespace -> path -> path_item_object schemas: Record> = {} // category -> schema -> schema_object - constructor (root_folder: string, logger: Logger = new Logger(LogLevel.warn)) { + constructor (root_folder: string, logger: Logger = new Logger()) { this.logger = logger this.root_folder = fs.realpathSync(root_folder) this.spec = { diff --git a/tools/src/merger/SupersededOpsGenerator.ts b/tools/src/merger/SupersededOpsGenerator.ts index 72f8ee5b6..0fd34eb54 100644 --- a/tools/src/merger/SupersededOpsGenerator.ts +++ b/tools/src/merger/SupersededOpsGenerator.ts @@ -10,14 +10,14 @@ import { type OperationSpec, type SupersededOperationMap } from 'types' import _ from 'lodash' import { read_yaml } from '../../helpers' -import { Logger, LogLevel } from '../Logger' +import { type Logger } from '../Logger' export default class SupersededOpsGenerator { logger: Logger superseded_ops: SupersededOperationMap - constructor (root_path: string, logger: Logger | undefined) { - this.logger = logger ?? new Logger(LogLevel.warn) + constructor (root_path: string, logger: Logger) { + this.logger = logger const file_path = root_path + '/_superseded_operations.yaml' this.superseded_ops = read_yaml(file_path) delete this.superseded_ops.$schema diff --git a/tools/src/tester/start.ts b/tools/src/tester/start.ts index 41a9eb8ad..b99d40201 100644 --- a/tools/src/tester/start.ts +++ b/tools/src/tester/start.ts @@ -8,7 +8,7 @@ */ import OpenApiMerger from '../merger/OpenApiMerger' -import { LogLevel } from '../Logger' +import { LogLevel, Logger } from '../Logger' import TestRunner from './TestRunner' import { Command, Option } from '@commander-js/extra-typings' import { @@ -45,8 +45,9 @@ const command = new Command() .parse() const opts = command.opts() +const logger = new Logger(opts.verbose ? LogLevel.info : LogLevel.warn) -const spec = (new OpenApiMerger(opts.specPath, LogLevel.error)).merge() +const spec = (new OpenApiMerger(opts.specPath, logger)).merge() const http_client = new OpenSearchHttpClient(get_opensearch_opts_from_cli(opts)) const chapter_reader = new ChapterReader(http_client) diff --git a/tools/tests/linter/SchemasValidator.test.ts b/tools/tests/linter/SchemasValidator.test.ts index 4713dc539..7bb99a945 100644 --- a/tools/tests/linter/SchemasValidator.test.ts +++ b/tools/tests/linter/SchemasValidator.test.ts @@ -7,10 +7,11 @@ * compatible open source license. */ +import { Logger } from 'Logger' import SchemasValidator from '../../src/linter/SchemasValidator' test('validate() - named_schemas', () => { - const validator = new SchemasValidator('./tools/tests/linter/fixtures/schemas_validator/named_schemas') + const validator = new SchemasValidator('./tools/tests/linter/fixtures/schemas_validator/named_schemas', new Logger()) expect(validator.validate()).toEqual([ { file: 'schemas/actions.yaml', @@ -26,7 +27,7 @@ test('validate() - named_schemas', () => { }) test('validate() - anonymous_schemas', () => { - const validator = new SchemasValidator('./tools/tests/linter/fixtures/schemas_validator/anonymous_schemas') + const validator = new SchemasValidator('./tools/tests/linter/fixtures/schemas_validator/anonymous_schemas', new Logger()) expect(validator.validate()).toEqual([ { file: '_global_parameters.yaml', diff --git a/tools/tests/linter/SpecValidator.test.ts b/tools/tests/linter/SpecValidator.test.ts index b62b13234..95557b0a2 100644 --- a/tools/tests/linter/SpecValidator.test.ts +++ b/tools/tests/linter/SpecValidator.test.ts @@ -7,10 +7,11 @@ * compatible open source license. */ +import { Logger } from 'Logger' import SpecValidator from 'linter/SpecValidator' test('validate()', () => { - const validator = new SpecValidator('./tools/tests/linter/fixtures/empty') + const validator = new SpecValidator('./tools/tests/linter/fixtures/empty', new Logger()) validator.namespaces_folder.validate = jest.fn().mockReturnValue([{ file: 'namespaces/', message: 'namespace error' }]) validator.schemas_folder.validate = jest.fn().mockReturnValue([{ file: 'schemas/', message: 'schema error' }]) validator.schema_refs_validator.validate = jest.fn().mockReturnValue([{ file: 'schema_refs', message: 'schema refs error' }]) diff --git a/tools/tests/linter/lint.test.ts b/tools/tests/linter/lint.test.ts new file mode 100644 index 000000000..7c9a012f6 --- /dev/null +++ b/tools/tests/linter/lint.test.ts @@ -0,0 +1,22 @@ +/* +* Copyright OpenSearch Contributors +* SPDX-License-Identifier: Apache-2.0 +* +* The OpenSearch Contributors require contributions made to +* this file be licensed under the Apache-2.0 license or a +* compatible open source license. +*/ + +import { spawnSync } from 'child_process' + +const spec = (args: string[]): any => { + const start = spawnSync('ts-node', ['tools/src/linter/lint.ts'].concat(args)) + return { + stdout: start.stdout?.toString(), + stderr: start.stderr?.toString() + } +} + +test('--help', () => { + expect(spec(['--help']).stdout).toContain('Usage: lint [options]') +}) diff --git a/tools/tests/merger/OpenApiMerger.test.ts b/tools/tests/merger/OpenApiMerger.test.ts index fdd2456a1..c8cd0e812 100644 --- a/tools/tests/merger/OpenApiMerger.test.ts +++ b/tools/tests/merger/OpenApiMerger.test.ts @@ -9,10 +9,10 @@ import OpenApiMerger from 'merger/OpenApiMerger' import fs from 'fs' -import { LogLevel } from '../../src/Logger' +import { Logger } from '../../src/Logger' test('merge()', () => { - const merger = new OpenApiMerger('./tools/tests/merger/fixtures/spec/', LogLevel.error) + const merger = new OpenApiMerger('./tools/tests/merger/fixtures/spec/', new Logger()) merger.merge('./tools/tests/merger/opensearch-openapi.yaml') expect(fs.readFileSync('./tools/tests/merger/fixtures/expected.yaml', 'utf8')) .toEqual(fs.readFileSync('./tools/tests/merger/opensearch-openapi.yaml', 'utf8')) diff --git a/tools/tests/merger/merge.test.ts b/tools/tests/merger/merge.test.ts new file mode 100644 index 000000000..e8499ea2f --- /dev/null +++ b/tools/tests/merger/merge.test.ts @@ -0,0 +1,22 @@ +/* +* Copyright OpenSearch Contributors +* SPDX-License-Identifier: Apache-2.0 +* +* The OpenSearch Contributors require contributions made to +* this file be licensed under the Apache-2.0 license or a +* compatible open source license. +*/ + +import { spawnSync } from 'child_process' + +const spec = (args: string[]): any => { + const start = spawnSync('ts-node', ['tools/src/merger/merge.ts'].concat(args)) + return { + stdout: start.stdout?.toString(), + stderr: start.stderr?.toString() + } +} + +test('--help', () => { + expect(spec(['--help']).stdout).toContain('Usage: merge [options]') +}) diff --git a/tools/tests/tester/start.test.ts b/tools/tests/tester/start.test.ts index 68b223153..47dcb6846 100644 --- a/tools/tests/tester/start.test.ts +++ b/tools/tests/tester/start.test.ts @@ -47,7 +47,10 @@ test('--dry-run', () => { const test_yaml = 'tools/tests/tester/fixtures/empty_with_all_the_parts.yaml' const s = spec(['--dry-run', '--tests', test_yaml]).stdout const full_path = path.join(__dirname, '../../../' + test_yaml) - expect(s).toEqual(`${ansi.yellow('SKIPPED')} ${ansi.cyan(ansi.b('A story with all its parts.'))} ${ansi.gray('(' + full_path + ')')}\n\n\n`) + expect(s).not.toContain(`${ansi.yellow('SKIPPED')} CHAPTERS`) + expect(s).not.toContain(`${ansi.yellow('SKIPPED')} EPILOGUES`) + expect(s).not.toContain(`${ansi.yellow('SKIPPED')} PROLOGUES`) + expect(s).toContain(`${ansi.yellow('SKIPPED')} ${ansi.cyan(ansi.b('A story with all its parts.'))} ${ansi.gray('(' + full_path + ')')}\n\n\n`) }) test('--dry-run --verbose', () => {