From f461433df9ae013e73a76a1103c9cb8a5d22ab52 Mon Sep 17 00:00:00 2001 From: Chris Campbell Date: Wed, 3 May 2023 14:31:35 -0700 Subject: [PATCH] fix: add inputId property to InputSpec interface (#321) Fixes #320 --- packages/build/docs/interfaces/InputSpec.md | 13 +++++ packages/build/src/_shared/model-spec.ts | 10 ++++ packages/plugin-config/src/context.ts | 9 +++- packages/plugin-config/src/gen-inputs.ts | 4 +- packages/plugin-config/src/processor.spec.ts | 55 +++++++++++++++++--- 5 files changed, 81 insertions(+), 10 deletions(-) diff --git a/packages/build/docs/interfaces/InputSpec.md b/packages/build/docs/interfaces/InputSpec.md index 9372af45..b6de2c19 100644 --- a/packages/build/docs/interfaces/InputSpec.md +++ b/packages/build/docs/interfaces/InputSpec.md @@ -6,6 +6,19 @@ Describes a model input variable. ## Properties +### inputId + + `Optional` **inputId**: `string` + +The stable input identifier. It is recommended to set this to a value (for example, a +numeric string like what `plugin-config` uses) that is separate from `varName` and is +stable between two versions of the model. This way, if an input variable is renamed +between two versions of the model, comparisons can still be performed between the two. +If a distinct `inputId` is not available, plugins can infer one from `varName`, but +note that this approach will be less resilient to renames. + +___ + ### varName **varName**: `string` diff --git a/packages/build/src/_shared/model-spec.ts b/packages/build/src/_shared/model-spec.ts index c5f7562b..f3fe5a86 100644 --- a/packages/build/src/_shared/model-spec.ts +++ b/packages/build/src/_shared/model-spec.ts @@ -4,6 +4,16 @@ * Describes a model input variable. */ export interface InputSpec { + /** + * The stable input identifier. It is recommended to set this to a value (for example, a + * numeric string like what `plugin-config` uses) that is separate from `varName` and is + * stable between two versions of the model. This way, if an input variable is renamed + * between two versions of the model, comparisons can still be performed between the two. + * If a distinct `inputId` is not available, plugins can infer one from `varName`, but + * note that this approach will be less resilient to renames. + */ + inputId?: string + /** The variable name (as used in the modeling tool). */ varName: string diff --git a/packages/plugin-config/src/context.ts b/packages/plugin-config/src/context.ts index b418df62..8b572dce 100644 --- a/packages/plugin-config/src/context.ts +++ b/packages/plugin-config/src/context.ts @@ -69,7 +69,13 @@ export class ConfigContext { this.buildContext.writeStagedFile(srcDir, dstDir, filename, content) } - addInputVariable(inputVarName: string, defaultValue: number, minValue: number, maxValue: number): void { + addInputVariable( + inputId: string, + inputVarName: string, + defaultValue: number, + minValue: number, + maxValue: number + ): void { // We use the C name as the key to avoid redundant entries in cases where // the csv file refers to variables with different capitalization const varId = sdeNameForVensimVarName(inputVarName) @@ -79,6 +85,7 @@ export class ConfigContext { console.error(`ERROR: Input variable ${inputVarName} was already added`) } this.inputSpecs.set(varId, { + inputId, varName: inputVarName, defaultValue, minValue, diff --git a/packages/plugin-config/src/gen-inputs.ts b/packages/plugin-config/src/gen-inputs.ts index 529dec79..9d723400 100644 --- a/packages/plugin-config/src/gen-inputs.ts +++ b/packages/plugin-config/src/gen-inputs.ts @@ -129,7 +129,7 @@ function inputSpecFromCsv(r: CsvRow, context: ConfigContext): InputSpec | undefi e += `default=${defaultValue} min=${minValue} max=${maxValue}` throw new Error(e) } - context.addInputVariable(varName, defaultValue, minValue, maxValue) + context.addInputVariable(inputId, varName, defaultValue, minValue, maxValue) const format = optionalString(r['format']) || '.0f' @@ -179,7 +179,7 @@ function inputSpecFromCsv(r: CsvRow, context: ConfigContext): InputSpec | undefi const minValue = Math.min(offValue, onValue) const maxValue = Math.max(offValue, onValue) - context.addInputVariable(varName, defaultValue, minValue, maxValue) + context.addInputVariable(inputId, varName, defaultValue, minValue, maxValue) // The `controlled input ids` field dictates which rows are active // when this switch is on or off. Examples of the format of this field: diff --git a/packages/plugin-config/src/processor.spec.ts b/packages/plugin-config/src/processor.spec.ts index 5c090e4a..0162f5f7 100644 --- a/packages/plugin-config/src/processor.spec.ts +++ b/packages/plugin-config/src/processor.spec.ts @@ -8,7 +8,7 @@ import { fileURLToPath } from 'url' import temp from 'temp' import { afterAll, beforeAll, describe, expect, it } from 'vitest' -import type { BuildOptions, UserConfig } from '@sdeverywhere/build' +import type { BuildOptions, ModelSpec, Plugin, UserConfig } from '@sdeverywhere/build' import { build } from '@sdeverywhere/build' import type { ConfigOptions } from './processor' @@ -22,7 +22,10 @@ interface TestEnv { buildOptions: BuildOptions } -async function prepareForBuild(optionsFunc: (corePkgDir: string) => ConfigOptions): Promise { +async function prepareForBuild( + optionsFunc: (corePkgDir: string) => ConfigOptions, + plugins: Plugin[] = [] +): Promise { const baseTmpDir = await temp.mkdir('sde-plugin-config') const projDir = joinPath(baseTmpDir, 'proj') await mkdir(projDir) @@ -32,7 +35,8 @@ async function prepareForBuild(optionsFunc: (corePkgDir: string) => ConfigOption const config: UserConfig = { rootDir: projDir, modelFiles: [], - modelSpec: configProcessor(optionsFunc(corePkgDir)) + modelSpec: configProcessor(optionsFunc(corePkgDir)), + plugins } const buildOptions: BuildOptions = { @@ -216,16 +220,53 @@ describe('configProcessor', () => { }) it('should write to default directory structure if single out dir is provided', async () => { + let capturedModelSpec: ModelSpec + const plugin: Plugin = { + async preGenerate(_, modelSpec) { + capturedModelSpec = modelSpec + } + } + const configDir = joinPath(__dirname, '__tests__', 'config1') - const testEnv = await prepareForBuild(corePkgDir => ({ - config: configDir, - out: corePkgDir - })) + const testEnv = await prepareForBuild( + corePkgDir => ({ + config: configDir, + out: corePkgDir + }), + [plugin] + ) const result = await build('production', testEnv.buildOptions) if (result.isErr()) { throw new Error('Expected ok result but got: ' + result.error.message) } + expect(capturedModelSpec).toBeDefined() + expect(capturedModelSpec.inputs).toEqual([ + { + inputId: '1', + varName: 'Input A', + defaultValue: 0, + minValue: -50, + maxValue: 50 + }, + { + inputId: '2', + varName: 'Input B', + defaultValue: 0, + minValue: -50, + maxValue: 50 + }, + { + inputId: '3', + varName: 'Input C', + defaultValue: 0, + minValue: 0, + maxValue: 1 + } + ]) + expect(capturedModelSpec.outputs).toEqual([{ varName: 'Var 1' }]) + expect(capturedModelSpec.datFiles).toEqual(['../Data1.dat', '../Data2.dat']) + const specJsonFile = joinPath(testEnv.projDir, 'sde-prep', 'spec.json') expect(await readFile(specJsonFile, 'utf8')).toEqual(specJson1)