From 6a9a90ee0254b3f98c3124eb290c26dcd97a7b2f Mon Sep 17 00:00:00 2001 From: dblock Date: Mon, 16 Dec 2024 12:22:24 -0500 Subject: [PATCH 1/3] Support combining multiple output variables. Signed-off-by: dblock --- CHANGELOG.md | 1 + TESTING_GUIDE.md | 4 ++- tools/src/tester/StoryEvaluator.ts | 10 ++----- tools/src/tester/StoryOutputs.ts | 13 +++++---- tools/src/tester/types/eval.types.ts | 38 +++++++++++++++++++++---- tools/tests/tester/StoryOutputs.test.ts | 10 +++++-- tools/tests/types/eval.types.test.ts | 23 +++++++++++++++ 7 files changed, 76 insertions(+), 23 deletions(-) create mode 100644 tools/tests/types/eval.types.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 60c0eb3df..f5464921e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Added `_type` to `termvector` and `mtermvector` ([#734](https://github.com/opensearch-project/opensearch-api-specification/pull/734)) - Added missing `cancelled` and `resource_stats` to `/_reindex/{task_id}/_rethrottle` ([#740](https://github.com/opensearch-project/opensearch-api-specification/pull/740)) - Added missing `cancellation_time_millis` to `POST /_tasks/_cancel` ([#747](https://github.com/opensearch-project/opensearch-api-specification/pull/747)) +- Added support for combining output variables ([#737](https://github.com/opensearch-project/opensearch-api-specification/pull/737)) ### Removed - Removed unsupported `_common.mapping:SourceField`'s `mode` field and associated `_common.mapping:SourceFieldMode` enum ([#652](https://github.com/opensearch-project/opensearch-api-specification/pull/652)) diff --git a/TESTING_GUIDE.md b/TESTING_GUIDE.md index 2812b37b2..c4590bd0a 100644 --- a/TESTING_GUIDE.md +++ b/TESTING_GUIDE.md @@ -244,7 +244,9 @@ Consider the following chapters in [ml/model_groups](tests/plugins/ml/ml/model_g ``` 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. -You can also supply defaults for output values, e.g. for `payload._version` used in [cluster/routing/awareness/weights.yaml](tests/routing/cluster/routing/awareness/weights.yaml). +You can combine multiple values, e.g. `task_id: ${task.node}:${task.id}`. + +You can supply defaults for output values, e.g. for `payload._version` used in [cluster/routing/awareness/weights.yaml](tests/routing/cluster/routing/awareness/weights.yaml). ``` version: diff --git a/tools/src/tester/StoryEvaluator.ts b/tools/src/tester/StoryEvaluator.ts index a17df942f..839760dc3 100644 --- a/tools/src/tester/StoryEvaluator.ts +++ b/tools/src/tester/StoryEvaluator.ts @@ -217,10 +217,7 @@ export default class StoryEvaluator { static #extract_params_variables(parameters: Record, variables: Set): void { Object.values(parameters ?? {}).forEach((param) => { if (typeof param === 'string') { - const ref = OutputReference.parse(param) - if (ref) { - variables.add(ref) - } + OutputReference.parse(param).forEach((ref) => variables.add(ref)) } }) } @@ -229,10 +226,7 @@ export default class StoryEvaluator { const request_type = typeof request switch (request_type) { case 'string': { - const ref = OutputReference.parse(request as string) - if (ref !== undefined) { - variables.add(ref) - } + OutputReference.parse(request as string).forEach((ref) => variables.add(ref)) break } case 'object': { diff --git a/tools/src/tester/StoryOutputs.ts b/tools/src/tester/StoryOutputs.ts index 41b8d9e71..838d66763 100644 --- a/tools/src/tester/StoryOutputs.ts +++ b/tools/src/tester/StoryOutputs.ts @@ -48,12 +48,13 @@ export class StoryOutputs { } resolve_string (str: string): any { - const ref = OutputReference.parse(str) - if (ref) { - return this.get_output_value(ref.chapter_id, ref.output_name) - } else { - return str - } + return OutputReference.replace(str, (chapter_id, output_name) => { + if (chapter_id === undefined || output_name === undefined) { + throw new Error(`Invalid output references in ${str}.`) + } + + return this.get_output_value(chapter_id as string, output_name as string) + }) } resolve_value (payload: any): any { diff --git a/tools/src/tester/types/eval.types.ts b/tools/src/tester/types/eval.types.ts index e65b80c3c..f29714afe 100644 --- a/tools/src/tester/types/eval.types.ts +++ b/tools/src/tester/types/eval.types.ts @@ -87,16 +87,42 @@ export enum Result { export class OutputReference { chapter_id: string output_name: string - private constructor (chapter_id: string, output_name: string) { + + private constructor(chapter_id: string, output_name: string) { this.chapter_id = chapter_id this.output_name = output_name } - static parse (str: string): OutputReference | undefined { - if (str.startsWith('${') && str.endsWith('}')) { - const spl = str.slice(2, -1).split('.', 2) - return { chapter_id: spl[0], output_name: spl[1] } + static parse(str: string): OutputReference[] { + const pattern = /\$\{([^}]+)\}/g + let match + var result = [] + while ((match = pattern.exec(str)) !== null) { + const spl = this.#split(match[1], '.', 2) + result.push(new OutputReference(spl[0], spl[1])) } - return undefined + return result + } + + static replace(str: string, callback: (chapter_id: any, variable: any) => string): any { + // preserve type if 1 value is returned + let full_match = str.match(/^\$\{([^}]+)\}$/) + if (full_match) { + const spl = this.#split(full_match[1], '.', 2) + return callback(spl[0], spl[1]) + } else return str.replace(/\$\{([^}]+)\}/g, (_, k) => { + const spl = this.#split(k, '.', 2) + return callback(spl[0], spl[1]) + }); + } + + static #split(str: any, delim: string, count: number): string[] { + if (str === undefined) return [str] + if (count <= 0) return [str] + const parts = str.split(delim) + if (parts.length <= count) return parts + const result = parts.slice(0, count - 1) + result.push(parts.slice(count - 1).join(delim)) + return result } } diff --git a/tools/tests/tester/StoryOutputs.test.ts b/tools/tests/tester/StoryOutputs.test.ts index 3017b7a66..044531328 100644 --- a/tools/tests/tester/StoryOutputs.test.ts +++ b/tools/tests/tester/StoryOutputs.test.ts @@ -55,13 +55,19 @@ test('resolve_params', () => { a: '${chapter_id.x}', b: '${chapter_id.y}', c: 3, - d: 'str' + d: 'str', + e: '${chapter_id.x}:${chapter_id.y}', + f: 'x=${chapter_id.x}', + g: '${chapter_id.y}=y' } expect(story_outputs.resolve_params(parameters)).toEqual({ a: 1, b: 2, c: 3, - d: 'str' + d: 'str', + e: '1:2', + f: 'x=1', + g: '2=y' }) }) diff --git a/tools/tests/types/eval.types.test.ts b/tools/tests/types/eval.types.test.ts new file mode 100644 index 000000000..76f7f3ac2 --- /dev/null +++ b/tools/tests/types/eval.types.test.ts @@ -0,0 +1,23 @@ +/* +* 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 { OutputReference } from "tester/types/eval.types"; + +describe('OutputReference', () => { + let f = (id: any, k: any): string => `[${id}:${k}]` + + describe('replace', () => { + it('replaces', () => { + expect(OutputReference.replace('string', f)).toEqual('string') + expect(OutputReference.replace('${k.v}', f)).toEqual('[k:v]') + expect(OutputReference.replace('${k.value}', f)).toEqual('[k:value]') + expect(OutputReference.replace('${k.v.m}', f)).toEqual('[k:v.m]') + }) + }) +}); From ff45ab995685d987c3f7b71960a6c40cad6a2424 Mon Sep 17 00:00:00 2001 From: dblock Date: Mon, 16 Dec 2024 12:35:02 -0500 Subject: [PATCH 2/3] Removed variable expansion from ML example. Signed-off-by: dblock --- .lycheeignore | 1 + tests/plugins/ml/ml/connectors.yaml | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.lycheeignore b/.lycheeignore index 1b6a61169..4c14ce320 100644 --- a/.lycheeignore +++ b/.lycheeignore @@ -1,3 +1,4 @@ https://localhost:* http://localhost:* http://webhook:8080 +https://api.openai.com/v1/chat/completions diff --git a/tests/plugins/ml/ml/connectors.yaml b/tests/plugins/ml/ml/connectors.yaml index 201019c04..c19939634 100644 --- a/tests/plugins/ml/ml/connectors.yaml +++ b/tests/plugins/ml/ml/connectors.yaml @@ -33,10 +33,10 @@ chapters: actions: - action_type: predict method: POST - url: 'https://${parameters.endpoint}/v1/chat/completions' + url: https://api.openai.com/v1/chat/completions headers: - Authorization: 'Bearer ${credential.openAI_key}' - request_body: '{ "model": "${parameters.model}", "messages": ${parameters.messages} }' + Authorization: Bearer Key + request_body: '{ "model": "model", "messages": "messages" }' response: status: 200 output: From 43dc5f2dbf9f89322ce9259b5026cf110cd47b54 Mon Sep 17 00:00:00 2001 From: dblock Date: Wed, 18 Dec 2024 09:59:31 -0500 Subject: [PATCH 3/3] Refactored split and OutputReference. Signed-off-by: dblock --- tools/src/helpers.ts | 18 +++++++ tools/src/tester/OutputReference.ts | 57 ++++++++++++++++++++++ tools/src/tester/StoryEvaluator.ts | 3 +- tools/src/tester/StoryOutputs.ts | 6 +-- tools/src/tester/types/eval.types.ts | 43 ---------------- tools/tests/helpers.test.ts | 25 +++++++++- tools/tests/tester/OutputReference.test.ts | 39 +++++++++++++++ tools/tests/tester/StoryOutputs.test.ts | 4 ++ tools/tests/types/eval.types.test.ts | 23 --------- 9 files changed, 145 insertions(+), 73 deletions(-) create mode 100644 tools/src/tester/OutputReference.ts create mode 100644 tools/tests/tester/OutputReference.test.ts delete mode 100644 tools/tests/types/eval.types.test.ts diff --git a/tools/src/helpers.ts b/tools/src/helpers.ts index 4144afd31..d338f7960 100644 --- a/tools/src/helpers.ts +++ b/tools/src/helpers.ts @@ -160,3 +160,21 @@ export function write_json (file_path: string, content: any, replacer?: (this: a export async function sleep (ms: number): Promise { await new Promise((resolve) => setTimeout(resolve, ms)) } + +/** + * Returns a string split using a delimiter, but only up to a certain number of times, + * returning the remainder of the string as the last element if the result. + * + * @param str a string + * @param delim delimiter + * @param count max number of splits + * @returns an array of strings + */ +export function split(str: any, delim: string, count: number = 0): string[] { + if (str === undefined) return [] + const parts = str.split(delim) + if (count <= 0 || parts.length <= count) return parts + const result = parts.slice(0, count - 1) + result.push(parts.slice(count - 1).join(delim)) + return result +} diff --git a/tools/src/tester/OutputReference.ts b/tools/src/tester/OutputReference.ts new file mode 100644 index 000000000..a76fb6249 --- /dev/null +++ b/tools/src/tester/OutputReference.ts @@ -0,0 +1,57 @@ +/* +* 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 { split } from "../helpers" + +export class OutputReference { + chapter_id: string + output_name: string + + constructor(chapter_id: string, output_name: string) { + this.chapter_id = chapter_id + this.output_name = output_name + } + + /** + * Parses a string and returns a collection of output references that it may contain. + * + * @param str a string + * @returns an array of output references + */ + static parse(str: string): OutputReference[] { + const pattern = /\$\{([^}]+)\}/g + let match + var result = [] + while ((match = pattern.exec(str)) !== null) { + const spl = split(match[1], '.', 2) + result.push(new OutputReference(spl[0], spl[1])) + } + return result + } + + /** + * Replaces occurrences of output references. + * Takes special care of preserving the type of the reference value if the entire string is a reference. + * + * @param str a string that may contain output references + * @param callback a callback for fetching reference values + * @returns a string with output references replaced by their values + */ + static replace(str: string, callback: (chapter_id: any, variable: any) => string): any { + let full_match = str.match(/^\$\{([^}]+)\}$/) + if (full_match) { + // the entire string is a reference, preserve the type of the returned value + const spl = split(full_match[1], '.', 2) + return callback(spl[0], spl[1]) + } else return str.replace(/\$\{([^}]+)\}/g, (_, k) => { + const spl = split(k, '.', 2) + return callback(spl[0], spl[1]) + }) + } +} diff --git a/tools/src/tester/StoryEvaluator.ts b/tools/src/tester/StoryEvaluator.ts index 839760dc3..eb0bbf2f7 100644 --- a/tools/src/tester/StoryEvaluator.ts +++ b/tools/src/tester/StoryEvaluator.ts @@ -8,7 +8,7 @@ */ import { ChapterRequest, Parameter, SupplementalChapter } from './types/story.types' -import { type StoryFile, type ChapterEvaluation, Result, type StoryEvaluation, OutputReference } from './types/eval.types' +import { type StoryFile, type ChapterEvaluation, Result, type StoryEvaluation } from './types/eval.types' import type ChapterEvaluator from './ChapterEvaluator' import { overall_result } from './helpers' import { StoryOutputs } from './StoryOutputs' @@ -17,6 +17,7 @@ import { ChapterOutput } from './ChapterOutput' import * as semver from '../_utils/semver' import _ from 'lodash' import { ParsedChapter, ParsedStory } from './types/parsed_story.types' +import { OutputReference } from './OutputReference' export default class StoryEvaluator { private readonly _chapter_evaluator: ChapterEvaluator diff --git a/tools/src/tester/StoryOutputs.ts b/tools/src/tester/StoryOutputs.ts index 838d66763..b535bc62e 100644 --- a/tools/src/tester/StoryOutputs.ts +++ b/tools/src/tester/StoryOutputs.ts @@ -8,7 +8,7 @@ */ import { ChapterOutput } from './ChapterOutput' -import { OutputReference } from './types/eval.types' +import { OutputReference } from './OutputReference' import { type Parameter } from './types/story.types' export class StoryOutputs { @@ -49,10 +49,6 @@ export class StoryOutputs { resolve_string (str: string): any { return OutputReference.replace(str, (chapter_id, output_name) => { - if (chapter_id === undefined || output_name === undefined) { - throw new Error(`Invalid output references in ${str}.`) - } - return this.get_output_value(chapter_id as string, output_name as string) }) } diff --git a/tools/src/tester/types/eval.types.ts b/tools/src/tester/types/eval.types.ts index f29714afe..0686a6896 100644 --- a/tools/src/tester/types/eval.types.ts +++ b/tools/src/tester/types/eval.types.ts @@ -83,46 +83,3 @@ export enum Result { SKIPPED = 'SKIPPED', ERROR = 'ERROR', } - -export class OutputReference { - chapter_id: string - output_name: string - - private constructor(chapter_id: string, output_name: string) { - this.chapter_id = chapter_id - this.output_name = output_name - } - - static parse(str: string): OutputReference[] { - const pattern = /\$\{([^}]+)\}/g - let match - var result = [] - while ((match = pattern.exec(str)) !== null) { - const spl = this.#split(match[1], '.', 2) - result.push(new OutputReference(spl[0], spl[1])) - } - return result - } - - static replace(str: string, callback: (chapter_id: any, variable: any) => string): any { - // preserve type if 1 value is returned - let full_match = str.match(/^\$\{([^}]+)\}$/) - if (full_match) { - const spl = this.#split(full_match[1], '.', 2) - return callback(spl[0], spl[1]) - } else return str.replace(/\$\{([^}]+)\}/g, (_, k) => { - const spl = this.#split(k, '.', 2) - return callback(spl[0], spl[1]) - }); - } - - static #split(str: any, delim: string, count: number): string[] { - if (str === undefined) return [str] - if (count <= 0) return [str] - const parts = str.split(delim) - if (parts.length <= count) return parts - const result = parts.slice(0, count - 1) - result.push(parts.slice(count - 1).join(delim)) - return result - } -} diff --git a/tools/tests/helpers.test.ts b/tools/tests/helpers.test.ts index f91dde573..3e1f15476 100644 --- a/tools/tests/helpers.test.ts +++ b/tools/tests/helpers.test.ts @@ -8,7 +8,7 @@ */ import _ from 'lodash' -import { delete_matching_keys, find_refs, sort_array_by_keys, to_json, to_ndjson } from '../src/helpers' +import { delete_matching_keys, find_refs, sort_array_by_keys, to_json, to_ndjson, split } from '../src/helpers' describe('helpers', () => { describe('sort_array_by_keys', () => { @@ -195,4 +195,27 @@ describe('helpers', () => { })).toEqual(new Set(['#1', '#2', '#3', '#dup', '#/schemas/schema1', '#/schemas/schema2'])) }) }) + + describe('split', () => { + test('undefined', () => { + expect(split(undefined, '.')).toEqual([]) + }) + + test('one element', () => { + expect(split('str', '.')).toEqual(['str']) + expect(split('str', '.', -1)).toEqual(['str']) + expect(split('str', '.', 0)).toEqual(['str']) + expect(split('str', '.', 10)).toEqual(['str']) + }) + + test('multiple elements', () => { + expect(split('x.y.z', '.')).toEqual(['x', 'y', 'z']) + expect(split('x.y.z', '.', -1)).toEqual(['x', 'y', 'z']) + expect(split('x.y.z', '.', 0)).toEqual(['x', 'y', 'z']) + expect(split('x.y.z', '.', 1)).toEqual(['x.y.z']) + expect(split('x.y.z', '.', 2)).toEqual(['x', 'y.z']) + expect(split('x.y.z', '.', 3)).toEqual(['x', 'y', 'z']) + expect(split('x.y.z', '.', 4)).toEqual(['x', 'y', 'z']) + }) + }) }) diff --git a/tools/tests/tester/OutputReference.test.ts b/tools/tests/tester/OutputReference.test.ts new file mode 100644 index 000000000..76aa4a067 --- /dev/null +++ b/tools/tests/tester/OutputReference.test.ts @@ -0,0 +1,39 @@ +/* +* 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 { OutputReference } from "tester/OutputReference"; + +describe('OutputReference', () => { + let f = (id: any, k: any): string => `[${id}:${k}]` + + describe('parse', () => { + it('replaces', () => { + expect(OutputReference.parse('string')).toEqual([]) + expect(OutputReference.parse('${k.v}')).toEqual([new OutputReference('k', 'v')]) + expect(OutputReference.parse('${k.value}')).toEqual([new OutputReference('k', 'value')]) + expect(OutputReference.parse('${k.v.m}')).toEqual([new OutputReference('k', 'v.m')]) + expect(OutputReference.parse('A reference to ${k.v.m} and ${x.y}.')).toEqual([ + new OutputReference('k', 'v.m'), + new OutputReference('x', 'y') + ]) + }) + }) + + describe('replace', () => { + it('replaces', () => { + expect(OutputReference.replace('string', f)).toEqual('string') + expect(OutputReference.replace('${k.v}', f)).toEqual('[k:v]') + expect(OutputReference.replace('${k.value}', f)).toEqual('[k:value]') + expect(OutputReference.replace('${k.v.m}', f)).toEqual('[k:v.m]') + expect(OutputReference.replace('A reference to ${k.v.m} and ${x} and ${x.y}.', f)).toEqual( + 'A reference to [k:v.m] and [x:undefined] and [x:y].' + ) + }) + }) +}); diff --git a/tools/tests/tester/StoryOutputs.test.ts b/tools/tests/tester/StoryOutputs.test.ts index 044531328..9339a860b 100644 --- a/tools/tests/tester/StoryOutputs.test.ts +++ b/tools/tests/tester/StoryOutputs.test.ts @@ -19,7 +19,11 @@ const story_outputs = new StoryOutputs({ test('resolve_string', () => { expect(story_outputs.resolve_string('${chapter_id.x}')).toEqual(1) + expect(story_outputs.resolve_string('${chapter_id.y}')).toEqual(2) expect(story_outputs.resolve_string('${invalid_id.x}')).toBeUndefined() + expect(story_outputs.resolve_string('${invalid_id.y}')).toBeUndefined() + expect(story_outputs.resolve_string('${chapter_id.x} and ${chapter_id.y}')).toEqual('1 and 2') + expect(story_outputs.resolve_string('${chapter_id.x} and ${chapter_id.y} and ${invalid.invalid}')).toEqual('1 and 2 and undefined') expect(story_outputs.resolve_string('some_str')).toEqual('some_str') }) diff --git a/tools/tests/types/eval.types.test.ts b/tools/tests/types/eval.types.test.ts deleted file mode 100644 index 76f7f3ac2..000000000 --- a/tools/tests/types/eval.types.test.ts +++ /dev/null @@ -1,23 +0,0 @@ -/* -* 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 { OutputReference } from "tester/types/eval.types"; - -describe('OutputReference', () => { - let f = (id: any, k: any): string => `[${id}:${k}]` - - describe('replace', () => { - it('replaces', () => { - expect(OutputReference.replace('string', f)).toEqual('string') - expect(OutputReference.replace('${k.v}', f)).toEqual('[k:v]') - expect(OutputReference.replace('${k.value}', f)).toEqual('[k:value]') - expect(OutputReference.replace('${k.v.m}', f)).toEqual('[k:v.m]') - }) - }) -});