From 9d4b1abd16edd38ee25f2fdedee44a729b979e7a Mon Sep 17 00:00:00 2001 From: joluj Date: Thu, 21 Dec 2023 18:36:47 +0100 Subject: [PATCH 01/10] Move model parsing to external method --- libs/interpreter-lib/src/interpreter.ts | 66 ++++++++++++++++++------- 1 file changed, 49 insertions(+), 17 deletions(-) diff --git a/libs/interpreter-lib/src/interpreter.ts b/libs/interpreter-lib/src/interpreter.ts index e63bac142..7cb516bfb 100644 --- a/libs/interpreter-lib/src/interpreter.ts +++ b/libs/interpreter-lib/src/interpreter.ts @@ -4,38 +4,39 @@ import { strict as assert } from 'assert'; +import * as R from '@jvalue/jayvee-execution'; import { - ExecutionContext, - Logger, - NONE, + DebugGranularity, executeBlocks, + ExecutionContext, isDebugGranularity, logExecutionDuration, + Logger, + NONE, parseValueToInternalRepresentation, registerDefaultConstraintExecutors, useExtension as useExecutionExtension, } from '@jvalue/jayvee-execution'; -import * as R from '@jvalue/jayvee-execution'; import { StdExecExtension } from '@jvalue/jayvee-extensions/std/exec'; import { BlockDefinition, - EvaluationContext, - JayveeModel, - JayveeServices, - PipelineDefinition, - RuntimeParameterProvider, collectChildren, collectStartingBlocks, createJayveeServices, + EvaluationContext, getBlocksInTopologicalSorting, initializeWorkspace, + JayveeModel, + JayveeServices, + PipelineDefinition, + RuntimeParameterProvider, } from '@jvalue/jayvee-language-server'; import * as chalk from 'chalk'; import { NodeFileSystem } from 'langium/node'; -import { LoggerFactory } from './logging/logger-factory'; +import { LoggerFactory } from './logging'; import { ExitCode, extractAstNodeFromString } from './parsing-util'; -import { validateRuntimeParameterLiteral } from './validation-checks/runtime-parameter-literal'; +import { validateRuntimeParameterLiteral } from './validation-checks'; interface InterpreterOptions { debugGranularity: R.DebugGranularity; @@ -48,6 +49,7 @@ export interface RunOptions { debug: boolean; debugGranularity: string; debugTarget: string | undefined; + parseOnly?: boolean; } export async function interpretString( @@ -66,13 +68,25 @@ export async function interpretString( return await interpretModel(extractAstNodeFn, options); } -export async function interpretModel( +/** + * Parses a model without executing it. + * Also sets up the environment so that the model can be properly executed. + * + * @returns non-null model, services and loggerFactory on success. + */ +export async function parseModel( extractAstNodeFn: ( services: JayveeServices, loggerFactory: LoggerFactory, ) => Promise, options: RunOptions, -): Promise { +): Promise<{ + model: JayveeModel | null; + loggerFactory: LoggerFactory; + services: JayveeServices | null; +}> { + let services: JayveeServices | null = null; + let model: JayveeModel | null = null; const loggerFactory = new LoggerFactory(options.debug); if (!isDebugGranularity(options.debugGranularity)) { loggerFactory @@ -83,23 +97,40 @@ export async function interpretModel( ', ', )}.`, ); - return ExitCode.FAILURE; + return { model, services, loggerFactory }; } useStdExtension(); registerDefaultConstraintExecutors(); - const services = createJayveeServices(NodeFileSystem).Jayvee; + services = createJayveeServices(NodeFileSystem).Jayvee; await initializeWorkspace(services); setupJayveeServices(services, options.env); - let model: JayveeModel; try { model = await extractAstNodeFn(services, loggerFactory); + return { model, services, loggerFactory }; } catch (e) { loggerFactory .createLogger() .logErr('Could not extract the AST node of the given model.'); + return { model, services, loggerFactory }; + } +} + +export async function interpretModel( + extractAstNodeFn: ( + services: JayveeServices, + loggerFactory: LoggerFactory, + ) => Promise, + options: RunOptions, +): Promise { + const { model, services, loggerFactory } = await parseModel( + extractAstNodeFn, + options, + ); + + if (model == null || services == null) { return ExitCode.FAILURE; } @@ -111,7 +142,8 @@ export async function interpretModel( loggerFactory, { debug: options.debug, - debugGranularity: options.debugGranularity, + // type of options.debugGranularity is asserted in parseModel + debugGranularity: options.debugGranularity as DebugGranularity, debugTargets: debugTargets, }, ); From 99c60c5ca6f55aad311bfeb6938f5f0a7d77521b Mon Sep 17 00:00:00 2001 From: joluj Date: Thu, 21 Dec 2023 18:40:19 +0100 Subject: [PATCH 02/10] add --parse-only flag --- apps/interpreter/src/index.ts | 5 +++ apps/interpreter/src/parse-only.spec.ts | 44 +++++++++++++++++++++++++ apps/interpreter/src/run-action.ts | 7 ++++ 3 files changed, 56 insertions(+) create mode 100644 apps/interpreter/src/parse-only.spec.ts diff --git a/apps/interpreter/src/index.ts b/apps/interpreter/src/index.ts index 5a7487b91..f0a1ba116 100644 --- a/apps/interpreter/src/index.ts +++ b/apps/interpreter/src/index.ts @@ -57,6 +57,11 @@ program `Sets the target blocks of the of block debug logging, separated by comma. If not given, all blocks are targeted.`, undefined, ) + .option( + '--parse-only', + 'Only parses the model without running it. Exits with 0 if the model is valid, with 1 otherwise.', + false, + ) .description('Run a Jayvee file') .action(runAction); diff --git a/apps/interpreter/src/parse-only.spec.ts b/apps/interpreter/src/parse-only.spec.ts new file mode 100644 index 000000000..1b298209a --- /dev/null +++ b/apps/interpreter/src/parse-only.spec.ts @@ -0,0 +1,44 @@ +import * as process from 'process'; +import { runAction } from './run-action'; +import { RunOptions } from '@jvalue/jayvee-interpreter-lib'; +import * as path from 'path'; + +describe('Parse Only', () => { + const baseDir = path.resolve(__dirname, '../../../example/'); + + const defaultOptions: RunOptions = { + env: new Map(), + debug: false, + debugGranularity: 'minimal', + debugTarget: undefined, + parseOnly: true, + }; + + beforeEach(() => { + jest.spyOn(process, 'exit').mockImplementation(() => { + throw new Error(); + }); + }); + + it('should exit with 0 on a valid option', async () => { + await expect( + runAction(path.resolve(baseDir, 'cars.jv'), { + ...defaultOptions, + }), + ).rejects.toBeDefined(); + + expect(process.exit).toBeCalledTimes(1); + expect(process.exit).toHaveBeenCalledWith(0); + }); + + it('should exit with 1 on error', async () => { + await expect( + runAction(path.resolve(baseDir, 'cars.jv'), { + ...defaultOptions, + }), + ).rejects.toBeDefined(); + + expect(process.exit).toBeCalledTimes(1); + expect(process.exit).toHaveBeenCalledWith(0); + }); +}); diff --git a/apps/interpreter/src/run-action.ts b/apps/interpreter/src/run-action.ts index a0bfa6faf..03b08391c 100644 --- a/apps/interpreter/src/run-action.ts +++ b/apps/interpreter/src/run-action.ts @@ -7,8 +7,10 @@ import { RunOptions, extractAstNodeFromFile, interpretModel, + parseModel, } from '@jvalue/jayvee-interpreter-lib'; import { JayveeModel, JayveeServices } from '@jvalue/jayvee-language-server'; +import * as process from 'process'; export async function runAction( fileName: string, @@ -23,6 +25,11 @@ export async function runAction( services, loggerFactory.createLogger(), ); + if (options.parseOnly) { + const { model, services } = await parseModel(extractAstNodeFn, options); + const exitCode = model && services ? 0 : 1; + process.exit(exitCode); + } const exitCode = await interpretModel(extractAstNodeFn, options); process.exit(exitCode); } From 3dccb3baf78156585f6c67fd5c22528320a382c4 Mon Sep 17 00:00:00 2001 From: joluj Date: Thu, 21 Dec 2023 18:48:09 +0100 Subject: [PATCH 03/10] add licence --- apps/interpreter/src/parse-only.spec.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/interpreter/src/parse-only.spec.ts b/apps/interpreter/src/parse-only.spec.ts index 1b298209a..970576558 100644 --- a/apps/interpreter/src/parse-only.spec.ts +++ b/apps/interpreter/src/parse-only.spec.ts @@ -1,3 +1,7 @@ +// SPDX-FileCopyrightText: 2023 Friedrich-Alexander-Universitat Erlangen-Nurnberg +// +// SPDX-License-Identifier: AGPL-3.0-only + import * as process from 'process'; import { runAction } from './run-action'; import { RunOptions } from '@jvalue/jayvee-interpreter-lib'; From 60fa5805ea7e7cd12e3c047ebae37fc56b0ea7d9 Mon Sep 17 00:00:00 2001 From: joluj Date: Thu, 21 Dec 2023 19:01:41 +0100 Subject: [PATCH 04/10] fix linter problems --- apps/interpreter/src/parse-only.spec.ts | 6 ++++-- apps/interpreter/src/run-action.ts | 5 +++-- libs/interpreter-lib/src/interpreter.ts | 16 ++++++++-------- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/apps/interpreter/src/parse-only.spec.ts b/apps/interpreter/src/parse-only.spec.ts index 970576558..2445dff7f 100644 --- a/apps/interpreter/src/parse-only.spec.ts +++ b/apps/interpreter/src/parse-only.spec.ts @@ -2,10 +2,12 @@ // // SPDX-License-Identifier: AGPL-3.0-only +import * as path from 'path'; import * as process from 'process'; -import { runAction } from './run-action'; + import { RunOptions } from '@jvalue/jayvee-interpreter-lib'; -import * as path from 'path'; + +import { runAction } from './run-action'; describe('Parse Only', () => { const baseDir = path.resolve(__dirname, '../../../example/'); diff --git a/apps/interpreter/src/run-action.ts b/apps/interpreter/src/run-action.ts index 03b08391c..7acb41a6e 100644 --- a/apps/interpreter/src/run-action.ts +++ b/apps/interpreter/src/run-action.ts @@ -2,6 +2,8 @@ // // SPDX-License-Identifier: AGPL-3.0-only +import * as process from 'process'; + import { LoggerFactory, RunOptions, @@ -10,7 +12,6 @@ import { parseModel, } from '@jvalue/jayvee-interpreter-lib'; import { JayveeModel, JayveeServices } from '@jvalue/jayvee-language-server'; -import * as process from 'process'; export async function runAction( fileName: string, @@ -25,7 +26,7 @@ export async function runAction( services, loggerFactory.createLogger(), ); - if (options.parseOnly) { + if (options.parseOnly === true) { const { model, services } = await parseModel(extractAstNodeFn, options); const exitCode = model && services ? 0 : 1; process.exit(exitCode); diff --git a/libs/interpreter-lib/src/interpreter.ts b/libs/interpreter-lib/src/interpreter.ts index 7cb516bfb..7fe7ac508 100644 --- a/libs/interpreter-lib/src/interpreter.ts +++ b/libs/interpreter-lib/src/interpreter.ts @@ -7,12 +7,12 @@ import { strict as assert } from 'assert'; import * as R from '@jvalue/jayvee-execution'; import { DebugGranularity, - executeBlocks, ExecutionContext, - isDebugGranularity, - logExecutionDuration, Logger, NONE, + executeBlocks, + isDebugGranularity, + logExecutionDuration, parseValueToInternalRepresentation, registerDefaultConstraintExecutors, useExtension as useExecutionExtension, @@ -20,16 +20,16 @@ import { import { StdExecExtension } from '@jvalue/jayvee-extensions/std/exec'; import { BlockDefinition, - collectChildren, - collectStartingBlocks, - createJayveeServices, EvaluationContext, - getBlocksInTopologicalSorting, - initializeWorkspace, JayveeModel, JayveeServices, PipelineDefinition, RuntimeParameterProvider, + collectChildren, + collectStartingBlocks, + createJayveeServices, + getBlocksInTopologicalSorting, + initializeWorkspace, } from '@jvalue/jayvee-language-server'; import * as chalk from 'chalk'; import { NodeFileSystem } from 'langium/node'; From 434ded98e24ee6e9170259df84447c3f21397a64 Mon Sep 17 00:00:00 2001 From: joluj Date: Fri, 29 Dec 2023 17:30:00 +0100 Subject: [PATCH 05/10] Add explicit null checks --- apps/interpreter/src/run-action.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/interpreter/src/run-action.ts b/apps/interpreter/src/run-action.ts index 7acb41a6e..2d9274f49 100644 --- a/apps/interpreter/src/run-action.ts +++ b/apps/interpreter/src/run-action.ts @@ -28,7 +28,7 @@ export async function runAction( ); if (options.parseOnly === true) { const { model, services } = await parseModel(extractAstNodeFn, options); - const exitCode = model && services ? 0 : 1; + const exitCode = model != null && services != null ? 0 : 1; process.exit(exitCode); } const exitCode = await interpretModel(extractAstNodeFn, options); From 262726214550d539306b1dbc462f018e0f4a9a77 Mon Sep 17 00:00:00 2001 From: joluj Date: Fri, 29 Dec 2023 17:30:54 +0100 Subject: [PATCH 06/10] Add -po shorthand for --parse-only --- apps/interpreter/src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/interpreter/src/index.ts b/apps/interpreter/src/index.ts index f0a1ba116..b5520eca5 100644 --- a/apps/interpreter/src/index.ts +++ b/apps/interpreter/src/index.ts @@ -58,7 +58,7 @@ program undefined, ) .option( - '--parse-only', + '-po, --parse-only', 'Only parses the model without running it. Exits with 0 if the model is valid, with 1 otherwise.', false, ) From f0e466383835fef2c28022619e20d5b01258e64f Mon Sep 17 00:00:00 2001 From: joluj Date: Fri, 29 Dec 2023 17:31:53 +0100 Subject: [PATCH 07/10] Fix test for exit-code 1 --- apps/interpreter/src/parse-only.spec.ts | 42 ++++++++++++++++++++++--- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/apps/interpreter/src/parse-only.spec.ts b/apps/interpreter/src/parse-only.spec.ts index 2445dff7f..af3a236fe 100644 --- a/apps/interpreter/src/parse-only.spec.ts +++ b/apps/interpreter/src/parse-only.spec.ts @@ -2,34 +2,56 @@ // // SPDX-License-Identifier: AGPL-3.0-only +import * as fs from 'node:fs/promises'; +import * as os from 'os'; import * as path from 'path'; import * as process from 'process'; +import { + clearBlockExecutorRegistry, + clearConstraintExecutorRegistry, +} from '@jvalue/jayvee-execution/test'; import { RunOptions } from '@jvalue/jayvee-interpreter-lib'; import { runAction } from './run-action'; describe('Parse Only', () => { const baseDir = path.resolve(__dirname, '../../../example/'); + const pathToValidModel = path.resolve(baseDir, 'cars.jv'); const defaultOptions: RunOptions = { env: new Map(), debug: false, debugGranularity: 'minimal', debugTarget: undefined, - parseOnly: true, }; + let tempFile: string | undefined = undefined; + + afterEach(async () => { + if (tempFile != null) { + await fs.rm(tempFile); + // eslint-disable-next-line require-atomic-updates + tempFile = undefined; + } + }); + beforeEach(() => { + jest.clearAllMocks(); jest.spyOn(process, 'exit').mockImplementation(() => { throw new Error(); }); + + // Reset jayvee specific stuff + clearBlockExecutorRegistry(); + clearConstraintExecutorRegistry(); }); it('should exit with 0 on a valid option', async () => { await expect( - runAction(path.resolve(baseDir, 'cars.jv'), { + runAction(pathToValidModel, { ...defaultOptions, + parseOnly: true, }), ).rejects.toBeDefined(); @@ -38,13 +60,25 @@ describe('Parse Only', () => { }); it('should exit with 1 on error', async () => { + const validModel = (await fs.readFile(pathToValidModel)).toString(); + + tempFile = path.resolve( + os.tmpdir(), + // E.g. "0.gn6v6ra9575" -> "gn6v6ra9575.jv" + Math.random().toString(36).substring(2) + '.jv', + ); + + // Write a partial valid model in that file + await fs.writeFile(tempFile, validModel.substring(validModel.length / 2)); + await expect( - runAction(path.resolve(baseDir, 'cars.jv'), { + runAction(tempFile, { ...defaultOptions, + parseOnly: true, }), ).rejects.toBeDefined(); expect(process.exit).toBeCalledTimes(1); - expect(process.exit).toHaveBeenCalledWith(0); + expect(process.exit).toHaveBeenCalledWith(1); }); }); From 400b2b471ecd70edcbf6e1f2489b9924634f3c1d Mon Sep 17 00:00:00 2001 From: joluj Date: Fri, 29 Dec 2023 17:40:11 +0100 Subject: [PATCH 08/10] Assert that models are not executed --- apps/interpreter/src/parse-only.spec.ts | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/apps/interpreter/src/parse-only.spec.ts b/apps/interpreter/src/parse-only.spec.ts index af3a236fe..0489225b4 100644 --- a/apps/interpreter/src/parse-only.spec.ts +++ b/apps/interpreter/src/parse-only.spec.ts @@ -11,10 +11,23 @@ import { clearBlockExecutorRegistry, clearConstraintExecutorRegistry, } from '@jvalue/jayvee-execution/test'; -import { RunOptions } from '@jvalue/jayvee-interpreter-lib'; +import { + RunOptions, + interpretModel, + interpretString, +} from '@jvalue/jayvee-interpreter-lib'; import { runAction } from './run-action'; +jest.mock('@jvalue/jayvee-interpreter-lib', () => { + const original: object = jest.requireActual('@jvalue/jayvee-interpreter-lib'); // Step 2. + return { + ...original, + interpretModel: jest.fn(), + interpretString: jest.fn(), + }; +}); + describe('Parse Only', () => { const baseDir = path.resolve(__dirname, '../../../example/'); const pathToValidModel = path.resolve(baseDir, 'cars.jv'); @@ -36,6 +49,12 @@ describe('Parse Only', () => { } }); + afterEach(() => { + // Assert that model is not executed + expect(interpretString).not.toBeCalled(); + expect(interpretModel).not.toBeCalled(); + }); + beforeEach(() => { jest.clearAllMocks(); jest.spyOn(process, 'exit').mockImplementation(() => { From fa661e0adab3754aa8bf437b46ebc26eeabbcf67 Mon Sep 17 00:00:00 2001 From: joluj Date: Tue, 9 Jan 2024 12:24:37 +0100 Subject: [PATCH 09/10] Make tests use handwritten falsy Jayvee model --- apps/interpreter/src/parse-only.spec.ts | 35 ++++---------- apps/interpreter/test/assets/broken-model.jv | 50 ++++++++++++++++++++ 2 files changed, 59 insertions(+), 26 deletions(-) create mode 100644 apps/interpreter/test/assets/broken-model.jv diff --git a/apps/interpreter/src/parse-only.spec.ts b/apps/interpreter/src/parse-only.spec.ts index 0489225b4..eb5abe1b1 100644 --- a/apps/interpreter/src/parse-only.spec.ts +++ b/apps/interpreter/src/parse-only.spec.ts @@ -2,8 +2,7 @@ // // SPDX-License-Identifier: AGPL-3.0-only -import * as fs from 'node:fs/promises'; -import * as os from 'os'; +import * as fs from 'node:fs'; import * as path from 'path'; import * as process from 'process'; @@ -20,7 +19,7 @@ import { import { runAction } from './run-action'; jest.mock('@jvalue/jayvee-interpreter-lib', () => { - const original: object = jest.requireActual('@jvalue/jayvee-interpreter-lib'); // Step 2. + const original: object = jest.requireActual('@jvalue/jayvee-interpreter-lib'); return { ...original, interpretModel: jest.fn(), @@ -29,8 +28,11 @@ jest.mock('@jvalue/jayvee-interpreter-lib', () => { }); describe('Parse Only', () => { - const baseDir = path.resolve(__dirname, '../../../example/'); - const pathToValidModel = path.resolve(baseDir, 'cars.jv'); + const pathToValidModel = path.resolve(__dirname, '../../../example/cars.jv'); + const pathToInvalidModel = path.resolve( + __dirname, + '../test/assets/broken-model.jv', + ); const defaultOptions: RunOptions = { env: new Map(), @@ -39,16 +41,6 @@ describe('Parse Only', () => { debugTarget: undefined, }; - let tempFile: string | undefined = undefined; - - afterEach(async () => { - if (tempFile != null) { - await fs.rm(tempFile); - // eslint-disable-next-line require-atomic-updates - tempFile = undefined; - } - }); - afterEach(() => { // Assert that model is not executed expect(interpretString).not.toBeCalled(); @@ -79,19 +71,10 @@ describe('Parse Only', () => { }); it('should exit with 1 on error', async () => { - const validModel = (await fs.readFile(pathToValidModel)).toString(); - - tempFile = path.resolve( - os.tmpdir(), - // E.g. "0.gn6v6ra9575" -> "gn6v6ra9575.jv" - Math.random().toString(36).substring(2) + '.jv', - ); - - // Write a partial valid model in that file - await fs.writeFile(tempFile, validModel.substring(validModel.length / 2)); + expect(fs.existsSync(pathToInvalidModel)).toBe(true); await expect( - runAction(tempFile, { + runAction(pathToInvalidModel, { ...defaultOptions, parseOnly: true, }), diff --git a/apps/interpreter/test/assets/broken-model.jv b/apps/interpreter/test/assets/broken-model.jv new file mode 100644 index 000000000..baeade010 --- /dev/null +++ b/apps/interpreter/test/assets/broken-model.jv @@ -0,0 +1,50 @@ +pipeline CarsPipeline { + // Try using a CoolCarsExtractor although we only have normal cars. + // This fill result in an error during parsing. + CoolCarsExtractor -> CarsTextFileInterpreter; + + CarsTextFileInterpreter + -> CarsCSVInterpreter + -> NameHeaderWriter + -> CarsTableInterpreter + -> CarsLoader; + + block CarsExtractor oftype HttpExtractor { + url: "https://gist.githubusercontent.com/noamross/e5d3e859aa0c794be10b/raw/b999fb4425b54c63cab088c0ce2c0d6ce961a563/cars.csv"; + } + + block CarsTextFileInterpreter oftype TextFileInterpreter { } + + block CarsCSVInterpreter oftype CSVInterpreter { + enclosing: '"'; + } + + block NameHeaderWriter oftype CellWriter { + at: cell A1; + write: ["name"]; + } + + block CarsTableInterpreter oftype TableInterpreter { + header: true; + columns: [ + "name" oftype text, + "mpg" oftype decimal, + "cyl" oftype integer, + "disp" oftype decimal, + "hp" oftype integer, + "drat" oftype decimal, + "wt" oftype decimal, + "qsec" oftype decimal, + "vs" oftype integer, + "am" oftype integer, + "gear" oftype integer, + "carb" oftype integer + ]; + } + + block CarsLoader oftype SQLiteLoader { + table: "Cars"; + file: "./cars.sqlite"; + } + +} \ No newline at end of file From 0598cde7d6ed54192aade0797bec43a82a2f6c3e Mon Sep 17 00:00:00 2001 From: joluj Date: Tue, 9 Jan 2024 12:25:27 +0100 Subject: [PATCH 10/10] Add license --- apps/interpreter/test/assets/broken-model.jv | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/interpreter/test/assets/broken-model.jv b/apps/interpreter/test/assets/broken-model.jv index baeade010..447eab7c2 100644 --- a/apps/interpreter/test/assets/broken-model.jv +++ b/apps/interpreter/test/assets/broken-model.jv @@ -1,3 +1,7 @@ +// SPDX-FileCopyrightText: 2024 Friedrich-Alexander-Universitat Erlangen-Nurnberg +// +// SPDX-License-Identifier: AGPL-3.0-only + pipeline CarsPipeline { // Try using a CoolCarsExtractor although we only have normal cars. // This fill result in an error during parsing.