diff --git a/integration-tests/cli/CHANGELOG.md b/integration-tests/cli/CHANGELOG.md index 9daeba9c9..16c0aea5f 100644 --- a/integration-tests/cli/CHANGELOG.md +++ b/integration-tests/cli/CHANGELOG.md @@ -1,5 +1,19 @@ # @openfn/integration-tests-cli +## 1.0.38 + +### Patch Changes + +- Updated dependencies [5f265bb] +- Updated dependencies [acb7b26] +- Updated dependencies [5f265bb] +- Updated dependencies [5f265bb] +- Updated dependencies [d52a52c] +- Updated dependencies [5f265bb] +- Updated dependencies [5f265bb] + - @openfn/project@0.23.1 + - @openfn/lightning-mock@2.5.8 + ## 1.0.37 ### Patch Changes diff --git a/integration-tests/cli/package.json b/integration-tests/cli/package.json index 74bf51651..585a3764c 100644 --- a/integration-tests/cli/package.json +++ b/integration-tests/cli/package.json @@ -1,7 +1,7 @@ { "name": "@openfn/integration-tests-cli", "private": true, - "version": "1.0.37", + "version": "1.0.38", "description": "CLI integration tests", "author": "Open Function Group ", "license": "ISC", diff --git a/integration-tests/cli/src/run.ts b/integration-tests/cli/src/run.ts index 9735d1305..35f134ae7 100644 --- a/integration-tests/cli/src/run.ts +++ b/integration-tests/cli/src/run.ts @@ -3,9 +3,12 @@ import * as path from 'node:path'; const isProd = process.env.NODE_ENV === 'production'; -const getOptions = () => ({ +const getOptions = (env = {}) => ({ env: { + // there's nobody to answer a prompt (a test can override this) + OPENFN_NO_CONFIRM: 'true', ...process.env, + ...env, OPENFN_REPO_DIR: path.resolve('repo'), // workspaces live inside the kit git repo: don't pick up its branch OPENFN_BRANCH: 'false', @@ -23,10 +26,12 @@ const mapOpenFnPath = (cmd) => { }; const run = async ( - cmd: string + cmd: string, + // extra environment variables for this command only + env?: Record ): Promise<{ stdout: string; stderr?: string; err?: any }> => { return new Promise((resolve) => { - exec(mapOpenFnPath(cmd), getOptions(), (err, stdout, stderr) => { + exec(mapOpenFnPath(cmd), getOptions(env), (err, stdout, stderr) => { resolve({ err, stdout, stderr }); }); }); diff --git a/integration-tests/cli/test/deploy.test.ts b/integration-tests/cli/test/deploy.test.ts index 39c605f73..d2887ad04 100644 --- a/integration-tests/cli/test/deploy.test.ts +++ b/integration-tests/cli/test/deploy.test.ts @@ -1,3 +1,5 @@ +// Tests legacy `openfn deploy` and `openfn pull` commands against a mock Lightning + import test from 'ava'; import path from 'node:path'; import fs, { rm } from 'node:fs/promises'; @@ -157,7 +159,6 @@ test.serial('deploy a local project', async (t) => { `openfn deploy \ --project-path ${tmpDir}/project.yaml \ --state-path ${tmpDir}/.state.json \ - --no-confirm \ --log-json \ -l debug` ); @@ -188,7 +189,6 @@ test.serial('Update a project', async (t) => { const deployCmd = `openfn deploy \ --project-path ${projectPath} \ --state-path ${statePath} \ - --no-confirm \ --log-json -l debug`; const first = await run(deployCmd); t.falsy(first.stderr); @@ -230,7 +230,6 @@ test.serial('pull a project', async (t) => { const deployCmd = `openfn deploy \ --project-path ${projectPath} \ --state-path ${statePath} \ - --no-confirm \ --log-json -l debug`; await run(deployCmd); @@ -323,7 +322,7 @@ test.serial('redirect to v2 protocol if openfn.yaml is present', async (t) => { // v1 deploy -> should redirect to v2 const { stdout, stderr } = await run( - `openfn deploy --workspace ${tmpDir} --no-confirm --log-json -l debug` + `openfn deploy --workspace ${tmpDir} --log-json -l debug` ); t.falsy(stderr); assertLog(t, extractLogs(stdout), /Detected openfn.yaml file/i); @@ -364,7 +363,6 @@ workflows: `openfn deploy \ --project-path ${tmpDir}/project.yaml \ --state-path ${tmpDir}/.state.json \ - --no-confirm \ --log-json \ -l debug` ); @@ -397,7 +395,6 @@ test.serial('deploy then pull, changes one workflow, deploy', async (t) => { const deployCmd = `openfn deploy \ --project-path ${projectPath} \ --state-path ${statePath} \ - --no-confirm \ --log debug \ --log-json`; await run(deployCmd); @@ -451,7 +448,6 @@ test.serial('deploy a v2 project.yaml', async (t) => { `openfn deploy \ --project-path ${tmpDir}/project.yaml \ --state-path ${tmpDir}/.state.json \ - --no-confirm \ --log-json \ -l debug` ); @@ -481,7 +477,6 @@ test.serial('deploy a new v2 project.yaml with credentials', async (t) => { const { stdout, stderr } = await run( `openfn deploy \ --project-path ${tmpDir}/project.yaml \ - --no-confirm \ --log-json \ -l debug` ); @@ -526,7 +521,6 @@ collections: const deployCmd = `openfn deploy \ --project-path ${projectPath} \ --state-path ${statePath} \ - --no-confirm \ --log-json -l debug`; // first deploy - no collections yet @@ -573,7 +567,6 @@ ${collectionsWorkflow} const deployCmd = `openfn deploy \ --project-path ${projectPath} \ --state-path ${statePath} \ - --no-confirm \ --log-json -l debug`; // first deploy - creates the collection diff --git a/integration-tests/cli/test/deploy.v2.test.ts b/integration-tests/cli/test/deploy.v2.test.ts index 7e5139dda..69c414849 100644 --- a/integration-tests/cli/test/deploy.v2.test.ts +++ b/integration-tests/cli/test/deploy.v2.test.ts @@ -1,3 +1,5 @@ +// Tests v2 `openfn project pull` and `openfn project deploy` loop against a mock Lightning + import test from 'ava'; import path from 'node:path'; import fs from 'node:fs/promises'; @@ -71,7 +73,7 @@ test.serial('pull, change and re-deploy', async (t) => { // deploy const { stdout, stderr } = await run( - `openfn project deploy --no-confirm --log-json -l debug` + `openfn project deploy --log-json -l debug` ); t.falsy(stderr); @@ -99,9 +101,7 @@ test.serial('pull, change and re-deploy twice', async (t) => { // first deploy await fs.writeFile(exprPath, 'fn(s => ({ ...s, v: 1 }))'); - const first = await run( - `openfn project deploy --no-confirm --log-json -l debug` - ); + const first = await run(`openfn project deploy --log-json -l debug`); t.falsy(first.stderr); assertLog(t, extractLogs(first.stdout), /Updated project/); @@ -112,7 +112,7 @@ test.serial('pull, change and re-deploy twice', async (t) => { // second deploy after another update await fs.writeFile(exprPath, 'fn(s => ({ ...s, v: 2 }))'); const { stdout, stderr } = await run( - `openfn project deploy --no-confirm --log-json -l debug` + `openfn project deploy --log-json -l debug` ); t.falsy(stderr); const logs = extractLogs(stdout); @@ -140,9 +140,7 @@ test.serial('deploy and pull to check version history', async (t) => { await fs.writeFile(exprPath, 'fn(s => ({ ...s, v: 1 }))'); // deploy then pull - const { stderr } = await run( - `openfn project deploy --no-confirm --log-json -l debug` - ); + const { stderr } = await run(`openfn project deploy --log-json -l debug`); t.falsy(stderr); await run(`openfn project pull ${projectId} --log-json -l debug`); @@ -174,7 +172,7 @@ test.serial('deploy then pull, change one workflow, deploy', async (t) => { // deploy const { stdout, stderr } = await run( - `openfn project deploy --no-confirm --log-json -l debug` + `openfn project deploy --log-json -l debug` ); t.falsy(stderr); const logs = extractLogs(stdout); @@ -215,7 +213,7 @@ test.serial('Remove a whole workflow', async (t) => { // deploy const { stdout, stderr } = await run( - `openfn project deploy --no-confirm --log-json -l debug` + `openfn project deploy --log-json -l debug` ); t.falsy(stderr); @@ -262,7 +260,7 @@ steps: await rimraf(path.join(tmpDir, 'workflows/my-workflow/my-job.js')); const { stdout, stderr } = await run( - `openfn project deploy --no-confirm --log-json -l debug` + `openfn project deploy --log-json -l debug` ); t.falsy(stderr); @@ -317,7 +315,7 @@ steps: ); const { stdout, stderr } = await run( - `openfn project deploy --no-confirm --log-json -l debug` + `openfn project deploy --log-json -l debug` ); t.falsy(stderr); @@ -372,9 +370,7 @@ test.serial.skip( await fs.writeFile(exprPath, "post('http://success.org')"); // deploy - const { stdout, stderr } = await run( - `openfn project deploy --no-confirm -l debug` - ); + const { stdout, stderr } = await run(`openfn project deploy -l debug`); console.log(stdout); t.falsy(stderr); const logs = extractLogs(stdout); @@ -400,9 +396,7 @@ test.serial('warn when local and remote workflows have diverged', async (t) => { // base await run(`openfn project pull ${projectId} --log-json -l debug`); await fs.writeFile(exprPath, 'fn(s => ({ ...s, v: 1 }))'); - const firstDeploy = await run( - `openfn project deploy --no-confirm --log-json -l debug` - ); + const firstDeploy = await run(`openfn project deploy --log-json -l debug`); assertLog(t, extractLogs(firstDeploy.stdout), /Updated project/); // remote changed from base @@ -424,7 +418,7 @@ test.serial('warn when local and remote workflows have diverged', async (t) => { // deploy with divergence const { stdout, err } = await run( - `openfn project deploy --no-confirm --log-json -l debug` + `openfn project deploy --log-json -l debug` ); t.truthy(err); const logs = extractLogs(stdout); @@ -448,7 +442,7 @@ test.serial('deploy a pulled v2 state file as a new project', async (t) => { // deploy that exact file back as a duplicate const { stdout, stderr } = await run( - `openfn project deploy ${pulledPath} --new --name my-duplicate --no-confirm --log-json -l debug` + `openfn project deploy ${pulledPath} --new --name my-duplicate --log-json -l debug` ); t.falsy(stderr); @@ -506,7 +500,7 @@ test.serial( // deploy main's pulled file, but target staging instead of main const { stdout, stderr } = await run( - `openfn project deploy ${mainPath} staging --no-confirm --log-json -l debug` + `openfn project deploy ${mainPath} staging --log-json -l debug` ); t.falsy(stderr); assertLog(t, extractLogs(stdout), /Updated project/); @@ -553,7 +547,7 @@ test.serial( await fs.writeFile(resourcesPath, jsonToYaml(before)); const { stdout, stderr } = await run( - `openfn project deploy --no-confirm --log-json -l debug` + `openfn project deploy --log-json -l debug` ); t.falsy(stderr); @@ -596,7 +590,7 @@ workflows: await fs.writeFile(exportedPath, specYaml); const { stdout, stderr } = await run( - `openfn project deploy ${exportedPath} --name my-duplicate --no-confirm --log-json -l debug` + `openfn project deploy ${exportedPath} --name my-duplicate --log-json -l debug` ); t.falsy(stderr); @@ -659,7 +653,7 @@ test.serial( await fs.writeFile(resourcesPath, jsonToYaml(before)); const { stdout, stderr } = await run( - `openfn project deploy --no-confirm --log-json -l debug` + `openfn project deploy --log-json -l debug` ); t.falsy(stderr); diff --git a/integration-tests/cli/test/project-v1.test.ts b/integration-tests/cli/test/project-v1.test.ts index 61aad365a..62ad835d8 100644 --- a/integration-tests/cli/test/project-v1.test.ts +++ b/integration-tests/cli/test/project-v1.test.ts @@ -1,3 +1,6 @@ +// Tests offline `openfn checkout`, `merge` and `projects` on static legacy v1 project yaml files +// (no Lightning); see project-v2 for the same commands on v2 files + import test from 'ava'; import { rm, mkdir, writeFile, readFile } from 'node:fs/promises'; import path from 'node:path'; diff --git a/integration-tests/cli/test/project-v2.test.ts b/integration-tests/cli/test/project-v2.test.ts index 5bd26a5d9..20285225c 100644 --- a/integration-tests/cli/test/project-v2.test.ts +++ b/integration-tests/cli/test/project-v2.test.ts @@ -1,3 +1,6 @@ +// Tests offline `openfn checkout`, `merge` and `projects` on static v2 project yaml files, +// plus executing a workflow from the checked out project; see project-v1 for the legacy format + import test from 'ava'; import { rm, mkdir, writeFile, readFile } from 'node:fs/promises'; import path from 'node:path'; diff --git a/integration-tests/cli/test/sync.test.ts b/integration-tests/cli/test/sync/commands.test.ts similarity index 86% rename from integration-tests/cli/test/sync.test.ts rename to integration-tests/cli/test/sync/commands.test.ts index 8291a6a64..a3415ad5f 100644 --- a/integration-tests/cli/test/sync.test.ts +++ b/integration-tests/cli/test/sync/commands.test.ts @@ -1,47 +1,15 @@ -import test from 'ava'; -import path from 'node:path'; -import fs from 'node:fs'; -import run from '../src/run'; -import { generateProject } from '@openfn/project'; -import createLightningServer from '@openfn/lightning-mock'; -import { rimraf } from 'rimraf'; - -let PORT = 5353; -let lightning; -const endpoint = `http://localhost:${PORT}/api/provision`; - -test.before(async () => { - await rimraf('tmp/sync'); - - lightning = createLightningServer({ - port: PORT, - }); -}); +// The v2 sync commands (fetch, pull and checkout) against a mock Lightning, +// asserting on what lands on disk (.projects state, checkout.yaml, workflows folder) -const initWorkspace = (t: any) => { - const id = t.title.replaceAll(' ', '_').toLowerCase(); - const p = path.resolve('tmp/sync', id); - - return { - workspace: p, - read: (filePath: string) => { - return fs.readFileSync(path.resolve(p, filePath), 'utf8'); - }, - }; -}; - -const gen = ( - name = 'patients', - workflows = ['trigger-job(expression="fn()")'] -) => { - // generate a project - const project = generateProject(name, workflows, { - openfnUuid: true, - }); - const state = project.serialize('state', { format: 'json' }); - lightning.addProject(state); - return project; -}; +import test from 'ava'; +import run from '../../src/run'; +import { setup } from './helpers'; + +const { lightning, endpoint, initWorkspace, gen } = setup( + test, + 'commands', + 5353 +); test('fetch a new project', async (t) => { const { workspace, read } = initWorkspace(t); diff --git a/integration-tests/cli/test/sync/conflict.test.ts b/integration-tests/cli/test/sync/conflict.test.ts new file mode 100644 index 000000000..3d6592670 --- /dev/null +++ b/integration-tests/cli/test/sync/conflict.test.ts @@ -0,0 +1,9 @@ +// Cases where the workspace and the remote project have both changed +// (not covered yet: these still need to be specified) + +import test from 'ava'; + +test.todo('rename in the app, then deploy a local edit without pulling'); +test.todo('rename a workflow differently in the app and locally'); +test.todo('delete a workflow in the app, then deploy a local edit to it'); +test.todo('add a workflow with the same name in the app and locally'); diff --git a/integration-tests/cli/test/sync/cross-project.test.ts b/integration-tests/cli/test/sync/cross-project.test.ts new file mode 100644 index 000000000..d70a2c573 --- /dev/null +++ b/integration-tests/cli/test/sync/cross-project.test.ts @@ -0,0 +1,226 @@ +// Deploying a workspace to a different project (eg, updating prod to match a project from +// the cloud, or replicating a project onto another server), against mock Lightnings: +// the target is made to look exactly like the workspace + +import test from 'ava'; +import { generateProject } from '@openfn/project'; +import createLightningServer from '@openfn/lightning-mock'; +import { setup } from './helpers'; + +const { lightning, initWorkspace, gen } = setup(test, 'cross-project', 5357); + +// a second server, for deploying across servers +const OTHER_PORT = 5358; +const otherEndpoint = `http://localhost:${OTHER_PORT}/api/provision`; +const other: any = createLightningServer({ port: OTHER_PORT }); + +// a project's workflows on a server, as { name: code } +const code = (server: any, project: any) => + Object.fromEntries( + ( + Object.values( + server.state.projects[project.openfn.uuid].workflows + ) as any[] + ).map((wf) => [ + wf.name, + Object.values(wf.jobs).find((j) => j.body).body, + ]) + ); + +// a project's workflows on a server, as { name: uuid } +const uuids = (server: any, project: any) => + Object.fromEntries( + ( + Object.values( + server.state.projects[project.openfn.uuid].workflows + ) as any[] + ).map((wf) => [wf.name, wf.id]) + ); + +// The workspace is pulled from cloud (Alpha and Beta). Prod has its own version of +// Alpha, and a workflow of its own (Gamma) +const genProjects = () => ({ + cloud: gen('cloud', [ + '@name Alpha trigger-job(expression="fn(a)")', + '@name Beta trigger-job(expression="fn(b)")', + ]), + prod: gen('prod', [ + '@name Alpha trigger-job(expression="fn(old)")', + '@name Gamma trigger-job(expression="fn(g)")', + ]), +}); + +test('deploy to a different project to make it match the workspace', async (t) => { + const { read, run } = initWorkspace(t); + const { cloud, prod } = genProjects(); + const prodAlpha = uuids(lightning, prod).Alpha; + + await run(`openfn project pull ${cloud.openfn.uuid}`); + + const { stdout } = await run( + `openfn project deploy ${prod.openfn.uuid} --force` + ); + t.regex(stdout, /Gamma: deleted/); + t.regex(stdout, /Alpha: changed/); + t.regex(stdout, /Beta: added/); + + // prod has exactly the workspace's workflows, and none of its own + t.deepEqual(code(lightning, prod), { Alpha: 'fn(a)', Beta: 'fn(b)' }); + + // its existing workflow was updated in place (it keeps its own uuid) + t.is(uuids(lightning, prod).Alpha, prodAlpha); + + // and cloud is untouched + t.deepEqual(code(lightning, cloud), { Alpha: 'fn(a)', Beta: 'fn(b)' }); + t.regex(read('workflows/alpha/alpha.yaml'), /name: Alpha/); +}); + +test('workflows deployed to a different project get new uuids', async (t) => { + const { run } = initWorkspace(t); + const { cloud, prod } = genProjects(); + + await run(`openfn project pull ${cloud.openfn.uuid}`); + await run(`openfn project deploy ${prod.openfn.uuid} --force`); + + const prodUuids = uuids(lightning, prod); + const cloudUuids = uuids(lightning, cloud); + + // Ensure prod's workflows have their own UUIDs + t.truthy(prodUuids.Alpha); + t.truthy(prodUuids.Beta); + t.not(prodUuids.Beta, cloudUuids.Beta); + t.not(prodUuids.Alpha, cloudUuids.Alpha); +}); + +test('deploy to a different project without a prompt will not remove workflows unless forced', async (t) => { + const { run } = initWorkspace(t); + const { cloud, prod } = genProjects(); + + await run(`openfn project pull ${cloud.openfn.uuid}`); + + // (the test runner never prompts, so this is a deploy with --no-confirm) + const { stdout, stderr, err } = await run( + `openfn project deploy ${prod.openfn.uuid}` + ); + t.truthy(err); + t.regex(stdout + stderr, /would remove workflows/); + t.regex(stdout + stderr, /gamma/); + + // prod is untouched + t.deepEqual(code(lightning, prod), { Alpha: 'fn(old)', Gamma: 'fn(g)' }); +}); + +test('a dry run to a different project shows what would change, and changes nothing', async (t) => { + const { run } = initWorkspace(t); + const cloud = gen('cloud', [ + '@name Alpha trigger-job(expression="fn(a)")', + '@name Beta trigger-job(expression="fn(b)")', + ]); + // a project with only a workflow of its own, which the deploy would remove + const prod = gen('prod', ['@name Gamma trigger-job(expression="fn(g)")']); + + await run(`openfn project pull ${cloud.openfn.uuid}`); + + // (without --force: a dry run isn't blocked by the removal guard) + const { stdout, err } = await run( + `openfn project deploy ${prod.openfn.uuid} --dry-run` + ); + t.falsy(err); + t.regex(stdout, /Gamma: deleted/); + t.regex(stdout, /Alpha: added/); + t.regex(stdout, /Beta: added/); + t.regex(stdout, /dryRun option set/); + + t.deepEqual(code(lightning, prod), { Gamma: 'fn(g)' }); +}); + +test('deploy to a different project that was edited elsewhere needs --force', async (t) => { + const { run } = initWorkspace(t); + const cloud = gen('cloud', [ + '@name Alpha trigger-job(expression="fn(a)")', + '@name Beta trigger-job(expression="fn(b)")', + ]); + // the same workflows, with different code (so no workflows are removed) + const prod = gen('prod', [ + '@name Alpha trigger-job(expression="fn(old-a)")', + '@name Beta trigger-job(expression="fn(old-b)")', + ]); + + await run(`openfn project pull ${cloud.openfn.uuid}`); + + const { stdout, stderr, err } = await run( + `openfn project deploy ${prod.openfn.uuid}` + ); + t.truthy(err); + t.regex(stdout + stderr, /not the project this workspace was synced from/); + t.deepEqual(code(lightning, prod), { + Alpha: 'fn(old-a)', + Beta: 'fn(old-b)', + }); + + await run(`openfn project deploy ${prod.openfn.uuid} --force`); + t.deepEqual(code(lightning, prod), { Alpha: 'fn(a)', Beta: 'fn(b)' }); +}); + +test('deploy to a different project by its local alias', async (t) => { + const { run } = initWorkspace(t); + const { cloud, prod } = genProjects(); + + await run(`openfn project pull ${cloud.openfn.uuid}`); + await run(`openfn project fetch --alias prod ${prod.openfn.uuid}`); + + await run('openfn project deploy prod --force'); + + t.deepEqual(code(lightning, prod), { Alpha: 'fn(a)', Beta: 'fn(b)' }); + t.deepEqual(code(lightning, cloud), { Alpha: 'fn(a)', Beta: 'fn(b)' }); +}); + +test('deploy a project yaml file to a different project', async (t) => { + const { workspace, run } = initWorkspace(t); + const { cloud, prod } = genProjects(); + + // save cloud to a file, rather than expanding it into a workspace + await run( + `openfn project fetch --output ${workspace}/cloud.yaml ${cloud.openfn.uuid}` + ); + + const { stdout } = await run( + `openfn project deploy cloud.yaml ${prod.openfn.uuid} --force` + ); + t.regex(stdout, /Gamma: deleted/); + t.regex(stdout, /Alpha: changed/); + t.regex(stdout, /Beta: added/); + + // prod matches the file, and cloud is untouched + t.deepEqual(code(lightning, prod), { Alpha: 'fn(a)', Beta: 'fn(b)' }); + t.deepEqual(code(lightning, cloud), { Alpha: 'fn(a)', Beta: 'fn(b)' }); +}); + +test('deploy a workspace to a project on a different server', async (t) => { + const { run } = initWorkspace(t); + const cloud = gen('cloud', [ + '@name Alpha trigger-job(expression="fn(a)")', + '@name Beta trigger-job(expression="fn(b)")', + ]); + + // a replica of cloud, on another server, with older code + const replica = generateProject( + 'replica', + ['@name Alpha trigger-job(expression="fn(old)")'], + { openfnUuid: true } + ); + other.addProject(replica.serialize('state', { format: 'json' })); + + await run(`openfn project pull ${cloud.openfn.uuid}`); + await run( + `openfn project deploy ${replica.openfn.uuid} --endpoint ${otherEndpoint} --force` + ); + + // the replica matches the workspace, and cloud is untouched + t.deepEqual(code(other, replica), { Alpha: 'fn(a)', Beta: 'fn(b)' }); + t.deepEqual(code(lightning, cloud), { Alpha: 'fn(a)', Beta: 'fn(b)' }); +}); + +test.todo( + 'after deploying to a different project, the workspace still deploys to the project it came from' +); diff --git a/integration-tests/cli/test/sync/edit.test.ts b/integration-tests/cli/test/sync/edit.test.ts new file mode 100644 index 000000000..551f5c639 --- /dev/null +++ b/integration-tests/cli/test/sync/edit.test.ts @@ -0,0 +1,97 @@ +// Adding, removing and editing workflows locally, then deploying them to a mock Lightning, +// checking both the remote project and the checkout files + +import test from 'ava'; +import path from 'node:path'; +import fs from 'node:fs'; +import { setup } from './helpers'; + +const { lightning, initWorkspace, gen } = setup(test, 'edit', 5355); + +test('add a workflow and deploy', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen('patients', [ + '@name Alpha trigger-job(expression="fn()")', + ]); + + await run(`openfn project pull ${project.openfn.uuid}`); + const [alphaBefore] = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + + // add a workflow, using the existing one as a starting point + fs.mkdirSync(path.join(workspace, 'workflows/beta')); + fs.writeFileSync( + path.join(workspace, 'workflows/beta/beta.yaml'), + read('workflows/alpha/alpha.yaml') + .replace('id: alpha', 'id: beta') + .replace('name: Alpha', 'name: Beta') + ); + fs.writeFileSync(path.join(workspace, 'workflows/beta/job.js'), 'fn(beta)'); + + const { stdout } = await run('openfn project deploy'); + t.regex(stdout, /Beta: added/); + + // the server has both workflows: the existing one is untouched, the new one has a uuid + const workflows = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + t.is(workflows.length, 2); + const alpha = workflows.find((w) => w.name === 'Alpha'); + const beta = workflows.find((w) => w.name === 'Beta'); + t.is(alpha.id, alphaBefore.id); + t.truthy(beta.id); + t.not(beta.id, alpha.id); + t.is(Object.values(beta.jobs)[0].body, 'fn(beta)'); + + // the checkout now tracks the new workflow, by the local id it was added with + const checkout = read('.openfn/checkout.yaml'); + t.regex( + checkout, + new RegExp(`workflow_uuids:\n alpha: ${alpha.id}\n beta: ${beta.id}`) + ); + + // everything is now in sync, so there is nothing left to deploy + const { stdout: again } = await run('openfn project deploy'); + t.regex(again, /Nothing to deploy/); +}); + +test('remove a workflow and deploy', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen('patients', [ + '@name Alpha trigger-job(expression="fn()")', + '@name Beta trigger-job(expression="fn()")', + ]); + + await run(`openfn project pull ${project.openfn.uuid}`); + const alphaBefore = ( + Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[] + ).find((w) => w.name === 'Alpha'); + + fs.rmSync(path.join(workspace, 'workflows/beta'), { recursive: true }); + + const { stdout } = await run('openfn project deploy'); + t.regex(stdout, /Beta: deleted/); + + // the workflow is gone from the server, and the other is untouched + const workflows = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + t.is(workflows.length, 1); + t.is(workflows[0].id, alphaBefore.id); + t.is(workflows[0].name, 'Alpha'); + + // and it's no longer tracked locally + const checkout = read('.openfn/checkout.yaml'); + t.regex( + checkout, + new RegExp(`workflow_uuids:\n alpha: ${alphaBefore.id}\n`) + ); + t.notRegex(checkout, /beta/); + + // everything is now in sync, so there is nothing left to deploy + const { stdout: again } = await run('openfn project deploy'); + t.regex(again, /Nothing to deploy/); +}); diff --git a/integration-tests/cli/test/sync/helpers.ts b/integration-tests/cli/test/sync/helpers.ts new file mode 100644 index 000000000..da9d74283 --- /dev/null +++ b/integration-tests/cli/test/sync/helpers.ts @@ -0,0 +1,62 @@ +// Shared setup for the sync tests +// +// ava runs test files in parallel, so each file gets its own mock Lightning +// (on its own port) and its own folder under tmp/sync + +import path from 'node:path'; +import fs from 'node:fs'; +import type { ExecutionContext, TestFn } from 'ava'; +import { generateProject } from '@openfn/project'; +import createLightningServer from '@openfn/lightning-mock'; +import { rimraf } from 'rimraf'; +import run from '../../src/run'; + +export const setup = (test: TestFn, name: string, port: number) => { + const root = path.resolve('tmp/sync', name); + const endpoint = `http://localhost:${port}/api/provision`; + + const lightning: any = createLightningServer({ port }); + + test.before(async () => { + await rimraf(root); + }); + + // Each test works in its own workspace, named after the test + const initWorkspace = (t: ExecutionContext) => { + const id = t.title.replaceAll(' ', '_').toLowerCase(); + const workspace = path.resolve(root, id); + + return { + workspace, + read: (filePath: string) => { + return fs.readFileSync(path.resolve(workspace, filePath), 'utf8'); + }, + // run a command against this workspace and server (and log it to the test output), without passing + // --workspace, --endpoint or --api-key (the env is set for this command only) + run: (cmd: string) => { + t.log(`$ ${cmd}`); + return run(cmd, { + OPENFN_WORKSPACE: workspace, + OPENFN_ENDPOINT: endpoint, + OPENFN_API_KEY: 'abc', + IGNORE_DOT_ENV: 'true', + }); + }, + }; + }; + + // generate a project and serve it from the mock Lightning + const gen = ( + name = 'patients', + workflows = ['trigger-job(expression="fn()")'] + ) => { + const project = generateProject(name, workflows, { + openfnUuid: true, + }); + const state = project.serialize('state', { format: 'json' }); + lightning.addProject(state); + return project; + }; + + return { lightning, endpoint, initWorkspace, gen }; +}; diff --git a/integration-tests/cli/test/sync/rename.test.ts b/integration-tests/cli/test/sync/rename.test.ts new file mode 100644 index 000000000..07fb4a164 --- /dev/null +++ b/integration-tests/cli/test/sync/rename.test.ts @@ -0,0 +1,570 @@ +// Renaming workflows locally, alone and combined with other edits, then deploying to a mock +// Lightning: the workflow must keep its uuid, and the checkout files must track its local id + +import test from 'ava'; +import path from 'node:path'; +import fs from 'node:fs'; +import { setup } from './helpers'; + +const { lightning, initWorkspace, gen } = setup(test, 'rename', 5356); + +test('rename a workflow and deploy', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen(); + const wfPath = path.join(workspace, 'workflows/workflow/workflow.yaml'); + + await run(`openfn project pull ${project.openfn.uuid}`); + + const [before] = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + const jobIds = Object.values(before.jobs).map((j) => j.id); + const triggerIds = Object.values(before.triggers).map((j) => j.id); + const edgeIds = Object.values(before.edges).map((j) => j.id); + + fs.writeFileSync( + wfPath, + read('workflows/workflow/workflow.yaml').replace( + 'name: Workflow', + 'name: Renamed' + ) + ); + + await run('openfn project deploy'); + + // the server has the same workflow and steps (by uuid), with the new name + const workflows = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + t.is(workflows.length, 1); + t.is(workflows[0].id, before.id); + t.is(workflows[0].name, 'Renamed'); + t.deepEqual( + Object.values(workflows[0].jobs).map((j) => j.id), + jobIds + ); + t.deepEqual( + Object.values(workflows[0].triggers).map((j) => j.id), + triggerIds + ); + t.deepEqual( + Object.values(workflows[0].edges).map((j) => j.id), + edgeIds + ); + + // the workflow stays where it was, under the id it had locally + t.regex(read('workflows/workflow/workflow.yaml'), /name: Renamed/); + t.false(fs.existsSync(path.join(workspace, 'workflows/renamed'))); + + // and the checkout file tracks it by that local id + const checkout = read('.openfn/checkout.yaml'); + t.regex(checkout, /forked_from:\n workflow: /); + t.regex(checkout, new RegExp(`workflow_uuids:\n workflow: ${before.id}`)); + t.notRegex(checkout, /renamed/); + + // everything is now in sync, so there is nothing left to deploy + const { stdout: again } = await run('openfn project deploy'); + t.regex(again, /Nothing to deploy/); +}); + +test('rename a workflow, deploy, then edit and deploy again', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen(); + const wfPath = path.join(workspace, 'workflows/workflow/workflow.yaml'); + + await run(`openfn project pull ${project.openfn.uuid}`); + const [before] = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + + fs.writeFileSync( + wfPath, + read('workflows/workflow/workflow.yaml').replace( + 'name: Workflow', + 'name: Renamed' + ) + ); + await run('openfn project deploy'); + + // now change the code in the renamed workflow and deploy again + fs.writeFileSync(path.join(workspace, 'workflows/workflow/job.js'), 'fn(x)'); + const { stdout } = await run('openfn project deploy'); + + // it's an edit to the workflow, not a new workflow + t.regex(stdout, /Renamed: changed/); + t.notRegex(stdout, /: (added|deleted)/); + + // the server workflow was updated in place + const workflows = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + t.is(workflows.length, 1); + t.is(workflows[0].id, before.id); + t.is(workflows[0].name, 'Renamed'); + t.is(Object.values(workflows[0].jobs)[0].body, 'fn(x)'); + + // and the checkout is still tracking the local id + const checkout = read('.openfn/checkout.yaml'); + t.regex(checkout, /forked_from:\n workflow: /); + t.regex(checkout, new RegExp(`workflow_uuids:\n workflow: ${before.id}`)); + + // everything is now in sync, so there is nothing left to deploy + const { stdout: again } = await run('openfn project deploy'); + t.regex(again, /Nothing to deploy/); +}); + +test('rename and edit a workflow in the same deploy', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen(); + const wfPath = path.join(workspace, 'workflows/workflow/workflow.yaml'); + + await run(`openfn project pull ${project.openfn.uuid}`); + const [before] = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + + fs.writeFileSync( + wfPath, + read('workflows/workflow/workflow.yaml').replace( + 'name: Workflow', + 'name: Renamed' + ) + ); + fs.writeFileSync(path.join(workspace, 'workflows/workflow/job.js'), 'fn(x)'); + + const { stdout } = await run('openfn project deploy'); + t.regex(stdout, /Renamed: changed/); + t.notRegex(stdout, /: (added|deleted)/); + + const workflows = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + t.is(workflows.length, 1); + t.is(workflows[0].id, before.id); + t.is(workflows[0].name, 'Renamed'); + t.is(Object.values(workflows[0].jobs)[0].body, 'fn(x)'); + + const checkout = read('.openfn/checkout.yaml'); + t.regex(checkout, new RegExp(`workflow_uuids:\n workflow: ${before.id}`)); + + // everything is now in sync, so there is nothing left to deploy + const { stdout: again } = await run('openfn project deploy'); + t.regex(again, /Nothing to deploy/); +}); + +test('rename two workflows, edit each, and deploy', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen('patients', [ + '@name Alpha trigger-job(expression="fn()")', + '@name Beta trigger-job(expression="fn()")', + ]); + + await run(`openfn project pull ${project.openfn.uuid}`); + const uuids: Record = {}; + for (const wf of Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]) { + uuids[wf.name] = wf.id; + } + + for (const [id, name] of [ + ['alpha', 'Alpha'], + ['beta', 'Beta'], + ]) { + const wfPath = path.join(workspace, `workflows/${id}/${id}.yaml`); + fs.writeFileSync( + wfPath, + read(`workflows/${id}/${id}.yaml`).replace( + `name: ${name}`, + `name: ${name} 2` + ) + ); + fs.writeFileSync( + path.join(workspace, `workflows/${id}/job.js`), + `fn(${id})` + ); + } + + await run('openfn project deploy'); + + // each workflow keeps its own uuid, with its own new name and code + const workflows = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + t.is(workflows.length, 2); + const alpha = workflows.find((w) => w.name === 'Alpha 2'); + const beta = workflows.find((w) => w.name === 'Beta 2'); + t.is(alpha.id, uuids.Alpha); + t.is(beta.id, uuids.Beta); + t.is(Object.values(alpha.jobs)[0].body, 'fn(alpha)'); + t.is(Object.values(beta.jobs)[0].body, 'fn(beta)'); + + // the checkout tracks both by their local ids + const checkout = read('.openfn/checkout.yaml'); + t.regex( + checkout, + new RegExp( + `workflow_uuids:\n alpha: ${uuids.Alpha}\n beta: ${uuids.Beta}` + ) + ); + + // everything is now in sync, so there is nothing left to deploy + const { stdout: again } = await run('openfn project deploy'); + t.regex(again, /Nothing to deploy/); +}); + +test('remove one workflow and rename another in the same deploy', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen('patients', [ + '@name Alpha trigger-job(expression="fn()")', + '@name Beta trigger-job(expression="fn()")', + ]); + + await run(`openfn project pull ${project.openfn.uuid}`); + const alphaBefore = ( + Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[] + ).find((w) => w.name === 'Alpha'); + + fs.rmSync(path.join(workspace, 'workflows/beta'), { recursive: true }); + fs.writeFileSync( + path.join(workspace, 'workflows/alpha/alpha.yaml'), + read('workflows/alpha/alpha.yaml').replace('name: Alpha', 'name: Alpha 2') + ); + + const { stdout } = await run('openfn project deploy'); + t.regex(stdout, /Beta: deleted/); + t.regex(stdout, /Alpha 2: changed/); + + // one workflow is left, with the same uuid and its new name + const workflows = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + t.is(workflows.length, 1); + t.is(workflows[0].id, alphaBefore.id); + t.is(workflows[0].name, 'Alpha 2'); + + // the checkout tracks only the survivor, by its local id + const checkout = read('.openfn/checkout.yaml'); + t.regex( + checkout, + new RegExp(`workflow_uuids:\n alpha: ${alphaBefore.id}\n`) + ); + t.notRegex(checkout, /beta/); + + // everything is now in sync, so there is nothing left to deploy + const { stdout: again } = await run('openfn project deploy'); + t.regex(again, /Nothing to deploy/); +}); + +test('rename a step and deploy', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen(); + const wfPath = path.join(workspace, 'workflows/workflow/workflow.yaml'); + + await run(`openfn project pull ${project.openfn.uuid}`); + const [before] = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + const jobIds = Object.values(before.jobs) + .map((j) => j.id) + .sort(); + const edgeIds = Object.values(before.edges).map((e) => e.id); + const jobId = Object.values(before.jobs).find( + (j) => j.name === 'job' + ).id; + + // rename the step (but not its id, or the file its code is in) + fs.writeFileSync( + wfPath, + read('workflows/workflow/workflow.yaml').replace( + ' name: job\n', + ' name: Renamed job\n' + ) + ); + + await run('openfn project deploy'); + + // the server has the same steps and edge (by uuid), with the new name + const workflows = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + t.is(workflows.length, 1); + t.is(workflows[0].id, before.id); + t.deepEqual( + Object.values(workflows[0].jobs) + .map((j) => j.id) + .sort(), + jobIds + ); + t.is( + Object.values(workflows[0].jobs).find((j) => j.id === jobId).name, + 'Renamed job' + ); + t.deepEqual( + Object.values(workflows[0].edges).map((e) => e.id), + edgeIds + ); + + // the step stays where it was locally + t.regex(read('workflows/workflow/workflow.yaml'), /name: Renamed job/); + t.is(read('workflows/workflow/job.js'), 'fn()'); + + // everything is now in sync, so there is nothing left to deploy + const { stdout: again } = await run('openfn project deploy'); + t.regex(again, /Nothing to deploy/); +}); + +// Skipped: step ids drift after a step rename +// The first deploy refetches the project, which derives the step id from its new name +// (renamed-job), but the workspace still has job. The next deploy sees job removed and +// renamed-job added, so the step is replaced and gets a new uuid +test.skip('rename a step, deploy, then edit and deploy again', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen(); + const wfPath = path.join(workspace, 'workflows/workflow/workflow.yaml'); + + await run(`openfn project pull ${project.openfn.uuid}`); + const [before] = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + const jobId = Object.values(before.jobs).find( + (j) => j.name === 'job' + ).id; + + fs.writeFileSync( + wfPath, + read('workflows/workflow/workflow.yaml').replace( + ' name: job\n', + ' name: Renamed job\n' + ) + ); + await run('openfn project deploy'); + + // now change the code in the renamed step and deploy again + fs.writeFileSync(path.join(workspace, 'workflows/workflow/job.js'), 'fn(x)'); + const { stdout } = await run('openfn project deploy'); + t.regex(stdout, /Workflow: changed/); + t.notRegex(stdout, /: (added|deleted)/); + + // the step was updated in place, not replaced + const workflows = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + t.is(workflows.length, 1); + const job = Object.values(workflows[0].jobs).find((j) => j.id === jobId); + t.is(job.name, 'Renamed job'); + t.is(job.body, 'fn(x)'); + t.is(Object.keys(workflows[0].jobs).length, 2); + + // everything is now in sync, so there is nothing left to deploy + const { stdout: again } = await run('openfn project deploy'); + t.regex(again, /Nothing to deploy/); +}); + +test('rename and edit a step in the same deploy', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen(); + const wfPath = path.join(workspace, 'workflows/workflow/workflow.yaml'); + + await run(`openfn project pull ${project.openfn.uuid}`); + const [before] = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + const jobId = Object.values(before.jobs).find( + (j) => j.name === 'job' + ).id; + + fs.writeFileSync( + wfPath, + read('workflows/workflow/workflow.yaml').replace( + ' name: job\n', + ' name: Renamed job\n' + ) + ); + fs.writeFileSync(path.join(workspace, 'workflows/workflow/job.js'), 'fn(x)'); + + const { stdout } = await run('openfn project deploy'); + t.regex(stdout, /Workflow: changed/); + t.notRegex(stdout, /: (added|deleted)/); + + const workflows = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + t.is(workflows.length, 1); + const job = Object.values(workflows[0].jobs).find((j) => j.id === jobId); + t.is(job.name, 'Renamed job'); + t.is(job.body, 'fn(x)'); + t.is(Object.keys(workflows[0].jobs).length, 2); + + // everything is now in sync, so there is nothing left to deploy + const { stdout: again } = await run('openfn project deploy'); + t.regex(again, /Nothing to deploy/); +}); + +test('rename a workflow and one of its steps in the same deploy', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen(); + const wfPath = path.join(workspace, 'workflows/workflow/workflow.yaml'); + + await run(`openfn project pull ${project.openfn.uuid}`); + const [before] = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + const jobId = Object.values(before.jobs).find( + (j) => j.name === 'job' + ).id; + + fs.writeFileSync( + wfPath, + read('workflows/workflow/workflow.yaml') + .replace('name: Workflow', 'name: Renamed') + .replace(' name: job\n', ' name: Renamed job\n') + ); + + await run('openfn project deploy'); + + // the same workflow and step (by uuid), with new names + const workflows = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + t.is(workflows.length, 1); + t.is(workflows[0].id, before.id); + t.is(workflows[0].name, 'Renamed'); + t.is( + Object.values(workflows[0].jobs).find((j) => j.id === jobId).name, + 'Renamed job' + ); + t.is(Object.keys(workflows[0].jobs).length, 2); + + // the checkout tracks the workflow by its local id + t.regex( + read('.openfn/checkout.yaml'), + new RegExp(`workflow_uuids:\n workflow: ${before.id}`) + ); + + // everything is now in sync, so there is nothing left to deploy + const { stdout: again } = await run('openfn project deploy'); + t.regex(again, /Nothing to deploy/); +}); + +test('rename a workflow in the app, then pull', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen(); + + await run(`openfn project pull ${project.openfn.uuid}`); + + // a file of the user's, next to the workflow + fs.writeFileSync( + path.join(workspace, 'workflows/workflow/notes.md'), + 'my notes' + ); + + // rename the workflow in the app + project.workflows[0].name = 'Renamed'; + project.workflows[0].workflow.name = 'Renamed'; + lightning.addProject(project.serialize('state', { format: 'json' })); + + // --force is needed because a rename also trips a false divergence warning + await run(`openfn project pull ${project.openfn.uuid} --force`); + + // there is one workflow, with its new name and the user's file alongside it + const folders = fs.readdirSync(path.join(workspace, 'workflows')); + const workflowFolders = folders.filter((folder) => + fs + .readdirSync(path.join(workspace, 'workflows', folder)) + .some((file) => file.endsWith('.yaml')) + ); + t.is(workflowFolders.length, 1); + const files = fs.readdirSync( + path.join(workspace, 'workflows', workflowFolders[0]) + ); + t.true(files.includes('notes.md')); + t.true(files.includes('job.js')); + t.regex( + read( + `workflows/${workflowFolders[0]}/${files.find((f) => + f.endsWith('.yaml') + )}` + ), + /name: Renamed/ + ); +}); + +test('rename a workflow locally, deploy, then pull', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen(); + const wfPath = path.join(workspace, 'workflows/workflow/workflow.yaml'); + + await run(`openfn project pull ${project.openfn.uuid}`); + + // a file of the user's, next to the workflow + fs.writeFileSync( + path.join(workspace, 'workflows/workflow/notes.md'), + 'my notes' + ); + + fs.writeFileSync( + wfPath, + read('workflows/workflow/workflow.yaml').replace( + 'name: Workflow', + 'name: Renamed' + ) + ); + await run('openfn project deploy'); + + // --force is needed because a rename also trips a false divergence warning + await run(`openfn project pull ${project.openfn.uuid} --force`); + + // there is one workflow, with its new name and the user's file alongside it + const folders = fs.readdirSync(path.join(workspace, 'workflows')); + const workflowFolders = folders.filter((folder) => + fs + .readdirSync(path.join(workspace, 'workflows', folder)) + .some((file) => file.endsWith('.yaml')) + ); + t.is(workflowFolders.length, 1); + const files = fs.readdirSync( + path.join(workspace, 'workflows', workflowFolders[0]) + ); + t.true(files.includes('notes.md')); + t.true(files.includes('job.js')); + t.regex( + read( + `workflows/${workflowFolders[0]}/${files.find((f) => + f.endsWith('.yaml') + )}` + ), + /name: Renamed/ + ); +}); + +// (--force is needed because a rename also trips a false divergence warning: a separate known issue) +test('rename an aliased workflow in the app, then pull', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen(); + + await run(`openfn project pull ${project.openfn.uuid}`); + + // alias the workflow, by renaming its folder + fs.renameSync( + path.join(workspace, 'workflows/workflow'), + path.join(workspace, 'workflows/wf') + ); + + // rename the workflow in the app + project.workflows[0].name = 'Renamed'; + project.workflows[0].workflow.name = 'Renamed'; + lightning.addProject(project.serialize('state', { format: 'json' })); + + await run(`openfn project pull ${project.openfn.uuid} --force`); + + // the rename is applied, and the alias is preserved + t.deepEqual(fs.readdirSync(path.join(workspace, 'workflows')), ['wf']); + t.regex(read('workflows/wf/workflow.yaml'), /name: Renamed/); + t.true(fs.existsSync(path.join(workspace, 'workflows/wf/job.js'))); + + // and the checkout still tracks the workflow by its local id + t.regex(read('.openfn/checkout.yaml'), /forked_from:\n workflow: /); +}); diff --git a/integration-tests/cli/test/sync/roundtrip.test.ts b/integration-tests/cli/test/sync/roundtrip.test.ts new file mode 100644 index 000000000..4259932d2 --- /dev/null +++ b/integration-tests/cli/test/sync/roundtrip.test.ts @@ -0,0 +1,70 @@ +// Sync round trips (pull, edit, deploy, pull again) against a mock Lightning, +// checking that the workspace and the remote project stay consistent + +import test from 'ava'; +import path from 'node:path'; +import fs from 'node:fs'; +import { setup } from './helpers'; + +const { lightning, initWorkspace, gen } = setup(test, 'roundtrip', 5354); + +test('pull, edit, deploy, then pull again', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen(); + + await run(`openfn project pull ${project.openfn.uuid}`); + + fs.writeFileSync(path.join(workspace, 'workflows/workflow/job.js'), 'fn(x)'); + + await run('openfn project deploy'); + + // the server has the edit + const [workflow] = Object.values( + lightning.state.projects[project.openfn.uuid].workflows + ) as any[]; + t.is(Object.values(workflow.jobs)[0].body, 'fn(x)'); + + // the deploy updates the checkout, tracking the workflow as it is on disk + const checkoutAfterDeploy = read('.openfn/checkout.yaml'); + t.regex(checkoutAfterDeploy, /forked_from:\n workflow: /); + t.regex(checkoutAfterDeploy, /workflow_uuids:\n workflow: /); + + await run(`openfn project pull ${project.openfn.uuid}`); + + // pulling what we just deployed changes nothing + t.is(read('workflows/workflow/job.js'), 'fn(x)'); + t.deepEqual(fs.readdirSync(path.join(workspace, 'workflows')), ['workflow']); + t.deepEqual( + fs.readdirSync(path.join(workspace, 'workflows/workflow')).sort(), + ['job.js', 'workflow.yaml'] + ); + t.is(read('.openfn/checkout.yaml'), checkoutAfterDeploy); +}); + +test('fetch only updates the tracked project', async (t) => { + const { workspace, read, run } = initWorkspace(t); + const project = gen(); + + await run(`openfn project pull ${project.openfn.uuid}`); + const checkoutBefore = read('.openfn/checkout.yaml'); + + // Make a local change + fs.writeFileSync( + path.join(workspace, 'workflows/workflow/job.js'), + 'fn(local)' + ); + + // And a contrasting remote change + project.workflows[0].steps[0].expression = 'fn(remote)'; + lightning.addProject(project.serialize('state', { format: 'json' })); + + // Now fetch + await run(`openfn project fetch ${project.openfn.uuid}`); + + // the tracked project has the remote change + t.regex(read('.projects/main@localhost.yaml'), /fn\(remote\)/); + + // but the workspace is untouched + t.is(read('workflows/workflow/job.js'), 'fn(local)'); + t.is(read('.openfn/checkout.yaml'), checkoutBefore); +}); diff --git a/integration-tests/worker/src/init.ts b/integration-tests/worker/src/init.ts index 978886c3a..24cf77456 100644 --- a/integration-tests/worker/src/init.ts +++ b/integration-tests/worker/src/init.ts @@ -53,7 +53,7 @@ export const initWorker = async ( lightning: `ws://localhost:${lightningPort}/worker`, secret: crypto.randomUUID(), collectionsVersion: '1.0.0', - messageTimeoutSeconds: 0.01, + messageTimeoutSeconds: 0.1, batchLogs: true, ...workerArgs, }); diff --git a/integration-tests/worker/test/integration.test.ts b/integration-tests/worker/test/integration.test.ts index 8ea364753..bde90d0c3 100644 --- a/integration-tests/worker/test/integration.test.ts +++ b/integration-tests/worker/test/integration.test.ts @@ -12,7 +12,7 @@ let engine; let engineLogger; let lightningPort; -test.before(async () => { +test.beforeEach(async () => { const keys = await generateKeys(); lightningPort = nextPort(); lightning = initLightning(lightningPort, keys.private); @@ -33,13 +33,10 @@ test.before(async () => { )); }); -test.afterEach(() => { +test.afterEach(async () => { engineLogger._reset(); -}); - -test.after(async () => { lightning.destroy(); - await worker.destroy(); + await worker.destroy({ immediate: true }); }); const createDummyWorker = () => { @@ -653,7 +650,7 @@ test.serial('use local adaptor versions from monorepo', (t) => { test.serial("Don't send adaptor logs to stdout", (t) => { return new Promise(async (done) => { // We have to create a new worker with a different repo for this one - await worker.destroy(); + await worker.destroy({ immediate: true }); ({ worker, engineLogger } = await createDummyWorker()); const message = 've have been expecting you meester bond'; @@ -692,7 +689,7 @@ test.serial( (t) => { return new Promise(async (done) => { // We want to create our own special worker here - await worker.destroy(); + await worker.destroy({ immediate: true }); ({ worker, engineLogger } = await createDummyWorker()); const attempt1 = { @@ -739,7 +736,7 @@ test.serial( test.serial('worker should exit if it has an invalid key', (t) => { return new Promise(async (done) => { if (!worker.destroyed) { - await worker.destroy(); + await worker.destroy({ immediate: true }); } // generate a new, invalid, public key @@ -792,7 +789,7 @@ test.serial('worker should exit if it has an invalid key', (t) => { test.serial('set a default timeout on the worker', (t) => { return new Promise(async (done) => { if (!worker.destroyed) { - await worker.destroy(); + await worker.destroy({ immediate: true }); } ({ worker } = await initWorker(lightningPort, { @@ -842,7 +839,7 @@ test.serial('set a default timeout on the worker', (t) => { test.serial('set a timeout on a run', (t) => { return new Promise(async (done) => { if (!worker.destroyed) { - await worker.destroy(); + await worker.destroy({ immediate: true }); } ({ worker } = await initWorker(lightningPort, { @@ -882,7 +879,7 @@ test.serial('set a timeout on a run', (t) => { test.serial('set a default payload limit on the worker', (t) => { return new Promise(async (done) => { if (!worker.destroyed) { - await worker.destroy(); + await worker.destroy({ immediate: true }); } ({ worker } = await initWorker( @@ -923,7 +920,7 @@ test.serial('set a default payload limit on the worker', (t) => { test.serial('override the worker payload through run options', (t) => { return new Promise(async (done) => { if (!worker.destroyed) { - await worker.destroy(); + await worker.destroy({ immediate: true }); } ({ worker } = await initWorker( @@ -965,7 +962,7 @@ test.serial('override the worker payload through run options', (t) => { test.serial('Redact logs which exceed the payload limit', (t) => { return new Promise(async (done) => { if (!worker.destroyed) { - await worker.destroy(); + await worker.destroy({ immediate: true }); } ({ worker } = await initWorker(lightningPort, { @@ -1006,7 +1003,7 @@ test.serial('Redact logs which exceed the payload limit', (t) => { test.serial("Don't return dataclips which exceed the payload limit", (t) => { return new Promise(async (done) => { if (!worker.destroyed) { - await worker.destroy(); + await worker.destroy({ immediate: true }); } ({ worker } = await initWorker(lightningPort, { @@ -1046,7 +1043,7 @@ test.serial( "Don't send job logs to stdout when job_log_level is set to none", (t) => { return new Promise(async (done) => { - await worker.destroy(); + await worker.destroy({ immediate: true }); ({ worker, engineLogger } = await createDummyWorker()); const message = 'log that will never exist'; diff --git a/packages/cli/CHANGELOG.md b/packages/cli/CHANGELOG.md index 3dd520ebe..d59f4e702 100644 --- a/packages/cli/CHANGELOG.md +++ b/packages/cli/CHANGELOG.md @@ -1,5 +1,26 @@ # @openfn/cli +## 1.45.0 + +### Minor Changes + +- acb7b26: Deploying a workspace to a different project now makes that project match it + +### Patch Changes + +- 1f74134: An explicit `--checkout` or `--no-checkout` on `project deploy` now overrides `OPENFN_NO_CHECKOUT_AFTER_DEPLOY` +- 5f265bb: Fix deploying an edit to a renamed workflow deleting it from Lightning +- 5f265bb: Show a renamed workflow as changed, not added and removed, in project diffs +- 5f265bb: Add OPENFN_NO_CONFIRM env var +- Updated dependencies [5f265bb] +- Updated dependencies [acb7b26] +- Updated dependencies [5f265bb] +- Updated dependencies [5f265bb] +- Updated dependencies [d52a52c] +- Updated dependencies [5f265bb] +- Updated dependencies [5f265bb] + - @openfn/project@0.23.1 + ## 1.44.0 ### Minor Changes diff --git a/packages/cli/README.md b/packages/cli/README.md index cc71f6a91..c67a3358b 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -266,7 +266,7 @@ openfn deploy - `-p, --project-path ` - path to the project file (defaults to `project.yaml`) - `-s, --state-path ` - path to the state file (defaults to `.state.json`) - `-c, --config, --config-path` - path to the config file (defaults to `.config.json`) -- `--no-confirm` - skip the confirmation prompt +- `--no-confirm` - skip the confirmation prompt (or set the `OPENFN_NO_CONFIRM=true` env var) ## Workflow Aliases diff --git a/packages/cli/package.json b/packages/cli/package.json index 5c2b1987d..fbd6169ed 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/cli", - "version": "1.44.0", + "version": "1.45.0", "description": "CLI devtools for the OpenFn toolchain", "repository": { "url": "https://github.com/OpenFn/kit" diff --git a/packages/cli/src/options.ts b/packages/cli/src/options.ts index a22f1d4c6..6c9729c70 100644 --- a/packages/cli/src/options.ts +++ b/packages/cli/src/options.ts @@ -245,12 +245,20 @@ export const confirm: CLIOption = { yargs: { alias: ['y'], boolean: true, - description: "Skip confirmation prompts (e.g. 'Are you sure?')", + description: + "Skip confirmation prompts (e.g. 'Are you sure?'). Use --no-confirm or OPENFN_NO_CONFIRM=true to skip", }, ensure: (opts) => { if ((opts as any).y) { opts.confirm = false; } + // An explicit --confirm or --no-confirm always beats the env var + if ( + opts.confirm === undefined && + process.env.OPENFN_NO_CONFIRM === 'true' + ) { + opts.confirm = false; + } setDefaultValue(opts, 'confirm', true); }, }; diff --git a/packages/cli/src/projects/checkout.ts b/packages/cli/src/projects/checkout.ts index 710ee90a6..57dd45721 100644 --- a/packages/cli/src/projects/checkout.ts +++ b/packages/cli/src/projects/checkout.ts @@ -1,5 +1,9 @@ import yargs from 'yargs'; -import Project, { Workspace } from '@openfn/project'; +import Project, { + matchWorkflows, + relabelWorkflows, + Workspace, +} from '@openfn/project'; import type l from '@openfn/lexicon'; import path from 'path'; import fs from 'fs'; @@ -174,6 +178,21 @@ export const handler = async (options: CheckoutOptions, logger?: Logger) => { } } + // The incoming project derives workflow ids from their names, so a workflow + // renamed elsewhere arrives with a new id. When checking out the project that is + // already checked out, keep the ids the workspace has (matched by uuid, id or + // name) so that a renamed workflow stays in its folder, with its files + if ( + localProject?.workflows.length && + tracked && + isSameProject(tracked, switchProject) + ) { + switchProject = relabelWorkflows( + switchProject, + matchWorkflows(switchProject.workflows, localProject.workflows) + ); + } + // Workflow aliases are just folder names, so carry them over from the // checked out project to keep renamed folders in place const incomingIds = new Set(switchProject.workflows.map((wf) => wf.id)); diff --git a/packages/cli/src/projects/deploy.ts b/packages/cli/src/projects/deploy.ts index 80e3d3dcd..45014beb1 100644 --- a/packages/cli/src/projects/deploy.ts +++ b/packages/cli/src/projects/deploy.ts @@ -1,12 +1,15 @@ import yargs from 'yargs'; import Project, { + matchWorkflows, MergeProjectOptions, + relabelWorkflows, RESOURCES_FILE, toResourceChannels, versionsEqual, + Workflow, Workspace, } from '@openfn/project'; -import { isEqual } from 'lodash-es'; +import { isEqual, uniq } from 'lodash-es'; import { mkdir, readFile, writeFile } from 'node:fs/promises'; import path from 'node:path'; @@ -19,6 +22,8 @@ import { serialize, getSerializePath, writeCheckoutFile, + updateForkedFrom, + invertMatches, findLocallyChangedWorkflows, AuthOptions, } from './util'; @@ -129,6 +134,9 @@ export const hasRemoteDiverged = ( const refs = local.cli.forked_from ?? {}; + // The remote id of a workflow can differ from the local one (eg, after a rename) + const matches = matchWorkflows(local.workflows, remote.workflows ?? []); + const filteredWorkflows = workflows.length ? local.workflows.filter((w) => workflows.includes(w.id)) : local.workflows; @@ -137,7 +145,9 @@ export const hasRemoteDiverged = ( for (const wf of filteredWorkflows) { if (wf.id in refs) { const forkedVersion = refs[wf.id]; - const remoteVersion = remote.getWorkflow(wf.id)?.history.at(-1); + const remoteVersion = remote + .getWorkflow(matches[wf.id] ?? wf.id) + ?.history.at(-1); if (remoteVersion) { if (!versionsEqual(forkedVersion, remoteVersion!)) { diverged ??= []; @@ -211,7 +221,8 @@ export type SyncResult = { // It returns a merged state object // // skipLocallyChangedCheck: merge every workflow in the source, not just those -// edited since its own last snapshot (that check is meaningless for a file) +// edited since its own last snapshot (that check is meaningless for a file, or +// when deploying to a different project) const syncProjects = async ( options: DeployOptions, config: Required, @@ -252,6 +263,8 @@ const syncProjects = async ( // this will actually happen later } + const isCrossProjectDeploy = localProject.uuid !== remoteProject.uuid; + let mergeCandidates: string[]; if (options.workflow?.length) { const missing = options.workflow.filter( @@ -268,6 +281,14 @@ const syncProjects = async ( } else if (skipLocallyChangedCheck) { // the source is the spec, so every workflow in it is a candidate mergeCandidates = localProject.workflows.map((w) => w.id); + if (isCrossProjectDeploy) { + // a different project should end up looking exactly like the source, + // so its own workflows are candidates too (to be removed) + mergeCandidates = uniq([ + ...mergeCandidates, + ...remoteProject.workflows.map((w) => w.id), + ]); + } } else { mergeCandidates = await findLocallyChangedWorkflows(ws, localProject); } @@ -283,6 +304,26 @@ const syncProjects = async ( return null; } + const removedWorkflows = isCrossProjectDeploy + ? workflowDiffs.filter((d) => d.type === 'removed').map((d) => d.id) + : []; + // Without a prompt nobody reviews the removals + if ( + removedWorkflows.length && + !options.confirm && + !options.force && + !options.dryRun + ) { + logger.error(`Error: Deploying would remove workflows from ${ + remoteProject.uuid + }: ${removedWorkflows.join(', ')} + + The target is a different project, so it will be made to look exactly like the local project. + + Pass --force to remove them anyway, or deploy without --no-confirm to review the changes first.`); + throw new Error('REMOVES_WORKFLOWS'); + } + // Ensure there's no divergence // Skip divergence testing if the remote has no history in its workflows @@ -308,9 +349,13 @@ const syncProjects = async ( `The following workflows have diverged: ${divergentWorkflows}` ); if (!options.force) { + const reason = + localProject.uuid === remoteProject.uuid + ? 'The remote project has been edited since the local project was branched.' + : `The target project ${remoteProject.uuid} is not the project this workspace was synced from (${localProject.uuid}), so its version history can't be compared.`; logger.error(`Error: Projects have diverged! - The remote project has been edited since the local project was branched. Changes may be lost. + ${reason} Changes may be lost. Pass --force to override this error and deploy anyway.`); const e: any = new Error('PROJECTS_DIVERGED'); @@ -346,6 +391,15 @@ const syncProjects = async ( } const merged = Project.merge(localProject, remoteProject!, mergeOptions); + // Flag target-only workflows as deleted: just leaving them out of the + // payload would not remove them from the app + if (removedWorkflows.length) { + const ids = new Set(removedWorkflows); + merged.workflows = merged.workflows.map((wf) => + ids.has(wf.id) ? new Workflow(wf.toJSON() as any).remove() : wf + ); + } + return { merged, remoteProject, locallyChangedWorkflows: mergeCandidates }; }; @@ -538,8 +592,9 @@ export async function handler(options: DeployOptions, logger: Logger) { localProject, { uuid: targetUuid!, endpoint }, logger, - // a file's own snapshot tells us nothing about the target - !!filePath + // a file's own snapshot, or a snapshot of a different project, tells us + // nothing about the target + !!filePath || localProject.uuid !== targetUuid ); if (!syncResult) { return; @@ -646,7 +701,24 @@ export async function handler(options: DeployOptions, logger: Logger) { const configPath = path.resolve(workspacePath, configData.path); await mkdir(path.dirname(configPath), { recursive: true }); await writeFile(configPath, configData.content); - await writeCheckoutFile(workspacePath, finalProject, options.branch); + + // The fetched project derives its workflow ids from their names, which + // can differ from the ids we use locally (eg, after a rename). The + // checkout file tracks workflows by their local ids, so it matches the + // workflows on disk + const localIds = invertMatches( + matchWorkflows( + merged.workflows.filter((wf) => !wf.isRemoved()), + finalProject.workflows + ) + ); + // (the tracked project state keeps the ids it was fetched with) + updateForkedFrom(finalProject); + await writeCheckoutFile( + workspacePath, + relabelWorkflows(finalProject, localIds), + options.branch + ); // Sync collections into resources.yaml (anything else in there is left alone) const resourcesPath = path.resolve(workspacePath, RESOURCES_FILE); diff --git a/packages/cli/src/projects/diff.ts b/packages/cli/src/projects/diff.ts index af32f8d25..4dbb6d46a 100644 --- a/packages/cli/src/projects/diff.ts +++ b/packages/cli/src/projects/diff.ts @@ -131,7 +131,7 @@ export const printRichDiff = ( if (changed.length > 0) { for (const diff of changed) { - const localWf = local.getWorkflow(diff.id); + const localWf = local.getWorkflow(diff.bId ?? diff.id); const remoteWf = remote.getWorkflow(diff.id); const label = localWf?.name || diff.id; logger.always(c.yellow(`${label}: changed`)); diff --git a/packages/cli/src/projects/options.ts b/packages/cli/src/projects/options.ts index cf266a8d5..855a09431 100644 --- a/packages/cli/src/projects/options.ts +++ b/packages/cli/src/projects/options.ts @@ -65,13 +65,13 @@ export const checkout: CLIOption = { name: 'checkout', yargs: { boolean: true, - default: true, description: 'Checkout the project after deploying. Use --no-checkout or OPENFN_NO_CHECKOUT_AFTER_DEPLOY=true to skip', }, ensure: (opts: any) => { - if (process.env.OPENFN_NO_CHECKOUT_AFTER_DEPLOY === 'true') { - opts.checkout = false; + // An explicit --checkout or --no-checkout always beats the env var + if (opts.checkout === undefined) { + opts.checkout = process.env.OPENFN_NO_CHECKOUT_AFTER_DEPLOY !== 'true'; } }, }; diff --git a/packages/cli/src/projects/util.ts b/packages/cli/src/projects/util.ts index a34e8d7d2..932eb1152 100644 --- a/packages/cli/src/projects/util.ts +++ b/packages/cli/src/projects/util.ts @@ -260,6 +260,10 @@ export const updateForkedFrom = (proj: Project) => { return proj; }; +// Swap the keys and values of a workflow id mapping (eg, from matchWorkflows) +export const invertMatches = (matches: Record) => + Object.fromEntries(Object.entries(matches).map(([a, b]) => [b, a])); + // Set up the checkout file for a project without touching anything else // (use this when the workflows are already on disk, like after a git clone) export const writeCheckoutFile = async ( diff --git a/packages/cli/test/options/ensure/confirm.test.ts b/packages/cli/test/options/ensure/confirm.test.ts new file mode 100644 index 000000000..22f97a367 --- /dev/null +++ b/packages/cli/test/options/ensure/confirm.test.ts @@ -0,0 +1,72 @@ +import test from 'ava'; +import { confirm, Opts } from '../../../src/options'; + +const ENV = 'OPENFN_NO_CONFIRM'; +const original = process.env[ENV]; + +test.afterEach.always(() => { + if (original === undefined) { + delete process.env[ENV]; + } else { + process.env[ENV] = original; + } +}); + +test.serial('confirm defaults to true', (t) => { + delete process.env[ENV]; + const opts = {} as Opts; + confirm.ensure!(opts); + + t.true(opts.confirm); +}); + +test.serial('-y sets confirm to false', (t) => { + delete process.env[ENV]; + const opts = { y: true } as unknown as Opts; + confirm.ensure!(opts); + + t.false(opts.confirm); +}); + +test.serial('env=true with no flag sets confirm to false', (t) => { + process.env[ENV] = 'true'; + const opts = {} as Opts; + confirm.ensure!(opts); + + t.false(opts.confirm); +}); + +test.serial('env=false with no flag leaves confirm true', (t) => { + process.env[ENV] = 'false'; + const opts = {} as Opts; + confirm.ensure!(opts); + + t.true(opts.confirm); +}); + +test.serial( + 'env=1 (truthy but not "true") with no flag leaves confirm true', + (t) => { + process.env[ENV] = '1'; + const opts = {} as Opts; + confirm.ensure!(opts); + + t.true(opts.confirm); + } +); + +test.serial('env=true with an explicit --confirm leaves confirm true', (t) => { + process.env[ENV] = 'true'; + const opts = { confirm: true } as Opts; + confirm.ensure!(opts); + + t.true(opts.confirm); +}); + +test.serial('env unset with --no-confirm sets confirm to false', (t) => { + delete process.env[ENV]; + const opts = { confirm: false } as Opts; + confirm.ensure!(opts); + + t.false(opts.confirm); +}); diff --git a/packages/cli/test/projects/checkout.test.ts b/packages/cli/test/projects/checkout.test.ts index 9bd5eb277..2e83ec576 100644 --- a/packages/cli/test/projects/checkout.test.ts +++ b/packages/cli/test/projects/checkout.test.ts @@ -256,6 +256,66 @@ test.serial( } ); +test.serial( + 'checkout: a workflow renamed in the tracked project keeps its folder and id', + async (t) => { + await checkoutHandler( + { command: 'project-checkout', project: 'my-project', workspace: '/ws' }, + logger + ); + // a file of the user's, next to the workflow + fs.writeFileSync('/ws/workflows/simple-workflow-main/notes.md', 'my notes'); + + // the workflow is renamed (but has the same uuid) in the tracked project + const trackedPath = '/ws/.projects/project@app.openfn.org.yaml'; + const tracked = yamlToJson(fs.readFileSync(trackedPath, 'utf8')); + tracked.workflows[0].name = 'Renamed workflow'; + fs.writeFileSync(trackedPath, jsonToYaml(tracked)); + + // (force: a rename also trips a false divergence warning, which is a separate issue) + await checkoutHandler( + { + command: 'project-checkout', + project: 'my-project', + workspace: '/ws', + force: true, + }, + logger + ); + + // the workflow stays where it was, under the id it had, with the new name + t.deepEqual( + fs.readdirSync('/ws/workflows').sort(), + ['another-workflow-main', 'simple-workflow-main'].sort() + ); + t.deepEqual( + fs.readdirSync('/ws/workflows/simple-workflow-main').sort(), + [ + 'notes.md', + 'simple-workflow-main.yaml', + 'transform-data-to-fhir-standard.js', + ].sort() + ); + t.regex( + fs.readFileSync( + '/ws/workflows/simple-workflow-main/simple-workflow-main.yaml', + 'utf8' + ), + /name: Renamed workflow/ + ); + + // and the checkout file tracks it by that id + const checkout = yamlToJson( + fs.readFileSync('/ws/.openfn/checkout.yaml', 'utf8') + ); + t.deepEqual(Object.keys(checkout.forked_from).sort(), [ + 'another-workflow-main', + 'simple-workflow-main', + ]); + t.is(checkout.workflow_uuids['simple-workflow-main'], 'wf-id-main'); + } +); + test.serial( 'checkout: drops an alias that clashes with an incoming workflow id', async (t) => { @@ -511,6 +571,8 @@ formats: id: staging name: Staging uuid: +workflow_uuids: + simple-workflow: wf1 ` ); diff --git a/packages/cli/test/projects/deploy.test.ts b/packages/cli/test/projects/deploy.test.ts index 6abddc2a2..14bc45c1e 100644 --- a/packages/cli/test/projects/deploy.test.ts +++ b/packages/cli/test/projects/deploy.test.ts @@ -348,6 +348,8 @@ test.serial( project: '/ws/.projects/main@localhost.yaml', target: 'dev', confirm: false, + // dev's own workflows get removed, which needs --force without a prompt + force: true, } as any, logger ); @@ -471,6 +473,60 @@ test.serial('deploy a change to a project', async (t) => { t.is(Object.keys(server.state.projects).length, 1); }); +// https://github.com/OpenFn/kit/issues/1594 +test.serial( + 'deploy an edit to a workflow after renaming it: the checkout tracks the local id', + async (t) => { + await setup(projectYaml); + const options = { + endpoint: ENDPOINT, + apiKey: 'test-api-key', + workspace: '/ws', + confirm: false, + } as any; + + const wfPath = '/ws/workflows/my-workflow/my-workflow.yaml'; + await writeFile( + wfPath, + fs + .readFileSync(wfPath, 'utf8') + .replace('name: My Workflow', 'name: My Workflow (renamed)') + ); + await deploy(options, logger); + + // the files haven't moved, so the checkout file tracks the workflow by its + // local id, not the id derived from its new name + t.true(fs.existsSync(wfPath)); + const checkout = yamlToJson(readCheckout()!) as any; + t.deepEqual(Object.keys(checkout.forked_from), ['my-workflow']); + t.deepEqual(checkout.workflow_uuids, { + 'my-workflow': '72ca3eb0-042c-47a0-a2a1-a545ed4a8406', + }); + + await writeFile('/ws/workflows/my-workflow/transform-data.js', 'log()'); + logger._reset(); + await deploy(options, logger); + + // the summary shows an edit to the existing workflow + t.truthy(logger._find('always', /My Workflow \(renamed\): changed/)); + t.falsy(logger._find('always', /: (added|deleted)/)); + + // the workflow was updated, not deleted + const workflows = Object.values( + server.state.projects[UUID].workflows + ) as any[]; + t.is(workflows.length, 1); + t.is(workflows[0].id, '72ca3eb0-042c-47a0-a2a1-a545ed4a8406'); + t.is(workflows[0].name, 'My Workflow (renamed)'); + t.is( + Object.values(workflows[0].jobs).find( + (j) => j.name === 'Transform data' + ).body, + 'log()' + ); + } +); + test.serial( 'Error if the remote and local workflows have diverged', async (t) => { @@ -1033,8 +1089,8 @@ test('printRichDiff: should report mix of added, changed, and removed workflows' const wf1 = generateWorkflow('@id a trigger-x'); const wf2Remote = generateWorkflow('@id b trigger-y'); const wf2Local = generateWorkflow('@id b trigger-different'); - const wf3 = generateWorkflow('@id c trigger-z'); - const wf4 = generateWorkflow('@id d trigger-w'); + const wf3 = generateWorkflow('@id c @name wf-c trigger-z'); + const wf4 = generateWorkflow('@id d @name wf-d trigger-w'); const local = new Project({ name: 'local', @@ -1092,6 +1148,31 @@ test('hasRemoteDiverged: 1 workflow, no diverged', (t) => { t.falsy(diverged); }); +test('hasRemoteDiverged: finds a renamed workflow in the remote by its uuid', (t) => { + const local = new Project({ + id: 'p', + workflows: [ + { id: 'my-workflow', name: 'Wf', steps: [], openfn: { uuid: 'uuid-1' } }, + ], + cli: { forked_from: { 'my-workflow': 'a' } }, + } as any); + const remote = new Project({ + id: 'p', + workflows: [ + { + id: 'my-workflow-renamed', + name: 'Wf (renamed)', + steps: [], + history: ['a', 'b'], + openfn: { uuid: 'uuid-1' }, + }, + ], + } as any); + + // the remote has moved on since the local was forked + t.deepEqual(hasRemoteDiverged(local, remote), ['my-workflow']); +}); + test('hasRemoteDiverged: 1 workflow, 1 diverged', (t) => { const local = { workflows: [ @@ -1531,6 +1612,109 @@ test.serial( } ); +const findWorkflow = (projectId: string, name: string) => + Object.values(server.state.projects[projectId].workflows).find( + (wf: any) => wf.name === name + ) as any; + +test.serial( + 'deploy: makes a different project look exactly like the local project', + async (t) => { + await server.addProject(two_workflows_yaml); + await setup(projectYaml); + + await deploy( + { + endpoint: ENDPOINT, + apiKey: 'test-api-key', + workspace: '/ws', + project: TWO_WORKFLOWS_UUID, + checkout: false, + confirm: false, + force: true, + } as any, + logger + ); + + const workflows = Object.values( + server.state.projects[TWO_WORKFLOWS_UUID].workflows + ) as any[]; + t.deepEqual( + workflows.map((wf) => wf.name), + ['My Workflow'] + ); + t.is(workflows[0].jobs['transform-data'].body, 'fn()'); + } +); + +test.serial( + 'deploy: refuses to remove workflows from a different project with --no-confirm, unless --force', + async (t) => { + await server.addProject(two_workflows_yaml); + await setup(projectYaml); + + await t.throwsAsync( + () => + deploy( + { + endpoint: ENDPOINT, + apiKey: 'test-api-key', + workspace: '/ws', + project: TWO_WORKFLOWS_UUID, + checkout: false, + confirm: false, + } as any, + logger + ), + { message: 'REMOVES_WORKFLOWS' } + ); + + // the target is untouched + t.truthy(findWorkflow(TWO_WORKFLOWS_UUID, 'Workflow A')); + t.truthy(findWorkflow(TWO_WORKFLOWS_UUID, 'Workflow B')); + t.falsy(findWorkflow(TWO_WORKFLOWS_UUID, 'My Workflow')); + } +); + +test.serial( + 'deploy: errors when a different target project has the same workflow, unless --force', + async (t) => { + const OTHER_UUID = 'a1b2c3d4-0000-4000-8000-000000000001'; + server.addProject({ ...myProject_v1, id: OTHER_UUID }); + const modified = JSON.parse( + JSON.stringify(server.state.projects[OTHER_UUID].workflows['my-workflow']) + ); + modified.jobs['transform-data'].body = 'each()'; + server.updateWorkflow(OTHER_UUID, modified); + + await setup(projectYaml); + + const options = { + endpoint: ENDPOINT, + apiKey: 'test-api-key', + workspace: '/ws', + project: OTHER_UUID, + checkout: false, + confirm: false, + } as any; + + await t.throwsAsync(() => deploy(options, logger), { + message: /PROJECTS_DIVERGED/, + }); + t.truthy( + logger._find('error', /not the project this workspace was synced from/) + ); + + await deploy({ ...options, force: true }, logger); + t.is( + server.state.projects[OTHER_UUID].workflows['my-workflow'].jobs[ + 'transform-data' + ].body, + 'fn()' + ); + } +); + test.serial( 'deploy: an explicit endpoint overrides the one in the project file', async (t) => { diff --git a/packages/cli/test/projects/options.test.ts b/packages/cli/test/projects/options.test.ts new file mode 100644 index 000000000..56025ce31 --- /dev/null +++ b/packages/cli/test/projects/options.test.ts @@ -0,0 +1,63 @@ +import test from 'ava'; +import yargs from 'yargs'; + +import { command } from '../../src/projects/deploy'; + +const ENV = 'OPENFN_NO_CHECKOUT_AFTER_DEPLOY'; +const original = process.env[ENV]; + +test.afterEach.always(() => { + if (original === undefined) { + delete process.env[ENV]; + } else { + process.env[ENV] = original; + } +}); + +// Parsing runs the command handler (ensure), so this returns the final options +const parse = (cmd: string) => + yargs() + .command(command as any) + .parse(cmd) as any; + +test.serial('checkout defaults to true', (t) => { + delete process.env[ENV]; + const options = parse('deploy'); + t.true(options.checkout); +}); + +test.serial('env=true with no flag skips checkout', (t) => { + process.env[ENV] = 'true'; + const options = parse('deploy'); + t.false(options.checkout); +}); + +test.serial('env=false with no flag checks out', (t) => { + process.env[ENV] = 'false'; + const options = parse('deploy'); + t.true(options.checkout); +}); + +test.serial('env=1 (truthy but not "true") with no flag checks out', (t) => { + process.env[ENV] = '1'; + const options = parse('deploy'); + t.true(options.checkout); +}); + +test.serial('env=true with --checkout checks out', (t) => { + process.env[ENV] = 'true'; + const options = parse('deploy --checkout'); + t.true(options.checkout); +}); + +test.serial('env=false with --no-checkout skips checkout', (t) => { + process.env[ENV] = 'false'; + const options = parse('deploy --no-checkout'); + t.false(options.checkout); +}); + +test.serial('env unset with --no-checkout skips checkout', (t) => { + delete process.env[ENV]; + const options = parse('deploy --no-checkout'); + t.false(options.checkout); +}); diff --git a/packages/lightning-mock/CHANGELOG.md b/packages/lightning-mock/CHANGELOG.md index a654734a8..89e2f6e1b 100644 --- a/packages/lightning-mock/CHANGELOG.md +++ b/packages/lightning-mock/CHANGELOG.md @@ -1,5 +1,18 @@ # @openfn/lightning-mock +## 2.5.8 + +### Patch Changes + +- Updated dependencies [5f265bb] +- Updated dependencies [acb7b26] +- Updated dependencies [5f265bb] +- Updated dependencies [5f265bb] +- Updated dependencies [d52a52c] +- Updated dependencies [5f265bb] +- Updated dependencies [5f265bb] + - @openfn/project@0.23.1 + ## 2.5.7 ### Patch Changes diff --git a/packages/lightning-mock/package.json b/packages/lightning-mock/package.json index 54bbb4667..d932ed00d 100644 --- a/packages/lightning-mock/package.json +++ b/packages/lightning-mock/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/lightning-mock", - "version": "2.5.7", + "version": "2.5.8", "private": true, "description": "A mock Lightning server", "main": "dist/index.js", diff --git a/packages/project/CHANGELOG.md b/packages/project/CHANGELOG.md index f771e68a3..ccada9449 100644 --- a/packages/project/CHANGELOG.md +++ b/packages/project/CHANGELOG.md @@ -1,5 +1,17 @@ # @openfn/project +## 0.23.1 + +### Patch Changes + +- 5f265bb: Track workflow uuids in the checkout file, and load them onto workflows +- acb7b26: Deploying a workspace to a different project now makes that project match it +- 5f265bb: Fix deploying an edit to a renamed workflow deleting it from Lightning +- 5f265bb: Show a renamed workflow as changed, not added and removed, in project diffs +- d52a52c: Fix workflow discovery for project directories containing square brackets. +- 5f265bb: Match workflows by uuid, then id, then name when merging projects +- 5f265bb: Workflow ids in workflow.yaml files are now ignored on load: the file name is used instead + ## 0.23.0 ### Minor Changes diff --git a/packages/project/package.json b/packages/project/package.json index f0ddc3158..a8889f0bf 100644 --- a/packages/project/package.json +++ b/packages/project/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/project", - "version": "0.23.0", + "version": "0.23.1", "description": "Read, serialize, replicate and sync OpenFn projects", "author": "Open Function Group ", "license": "ISC", diff --git a/packages/project/src/Project.ts b/packages/project/src/Project.ts index a4eb59943..e6040f5be 100644 --- a/packages/project/src/Project.ts +++ b/packages/project/src/Project.ts @@ -42,6 +42,8 @@ type CLIMeta = { version?: number; alias?: string | null; forked_from?: Record; + // uuid of each workflow, by local id. Tracked but not used + workflow_uuids?: Record; }; export class Project { diff --git a/packages/project/src/index.ts b/packages/project/src/index.ts index 924474356..6e2ed120c 100644 --- a/packages/project/src/index.ts +++ b/packages/project/src/index.ts @@ -1,10 +1,17 @@ import { Project, MultipleMatchingWorkflowsError } from './Project'; import { Workspace } from './Workspace'; +import Workflow from './Workflow'; import { yamlToJson, jsonToYaml } from './util/yaml'; export default Project; -export { Workspace, yamlToJson, jsonToYaml, MultipleMatchingWorkflowsError }; +export { + Workspace, + Workflow, + yamlToJson, + jsonToYaml, + MultipleMatchingWorkflowsError, +}; export { generateWorkflow, generateProject } from './gen/generator'; @@ -39,6 +46,9 @@ export { default as detectVersion } from './util/detect-version'; export type { MergeProjectOptions } from './merge/merge-project'; +export { default as matchWorkflows } from './merge/match-workflows'; +export { default as relabelWorkflows } from './util/relabel-workflows'; + export { mergeCollections } from './merge/merge-project'; export { toResourceChannels } from './util/resources'; diff --git a/packages/project/src/merge/match-workflows.ts b/packages/project/src/merge/match-workflows.ts new file mode 100644 index 000000000..050f707b0 --- /dev/null +++ b/packages/project/src/merge/match-workflows.ts @@ -0,0 +1,58 @@ +import Workflow from '../Workflow'; + +/** + * Pair up the workflows of a source project with those of a target + * + * A source workflow is matched to a target workflow by, in order: + * 1. uuid + * 2. id + * 3. name (only if it's unique among the workflows still unmatched on both sides) + * + * Matches are one-to-one. A match where the ids differ is a rename. + * + * Returns a map of source workflow id -> target workflow id. + * Unmatched workflows do not appear in it. + */ +export default function matchWorkflows( + source: Workflow[], + target: Workflow[] +): Record { + const matches: Record = {}; + const claimed = new Set(); // target ids + + const unmatched = () => source.filter((s) => !(s.id in matches)); + const available = () => target.filter((t) => !claimed.has(t.id)); + const claim = (s: Workflow, t: Workflow) => { + matches[s.id] = t.id; + claimed.add(t.id); + }; + + for (const s of unmatched()) { + const uuid = s.openfn?.uuid; + if (uuid) { + const t = available().find((t) => t.openfn?.uuid === uuid); + if (t) { + claim(s, t); + } + } + } + + for (const s of unmatched()) { + const t = available().find((t) => t.id === s.id); + if (t) { + claim(s, t); + } + } + + for (const s of unmatched()) { + if (s.name) { + const sources = unmatched().filter((x) => x.name === s.name); + const targets = available().filter((t) => t.name === s.name); + if (sources.length === 1 && targets.length === 1) { + claim(s, targets[0]); + } + } + } + + return matches; +} diff --git a/packages/project/src/merge/merge-project.ts b/packages/project/src/merge/merge-project.ts index af6b5f770..566049bf4 100644 --- a/packages/project/src/merge/merge-project.ts +++ b/packages/project/src/merge/merge-project.ts @@ -6,6 +6,7 @@ import { } from '@openfn/lexicon'; import { Project } from '../Project'; import { mergeWorkflows } from './merge-workflow'; +import matchWorkflows from './match-workflows'; import mapUuids from './map-uuids'; import baseMerge from '../util/base-merge'; import getDuplicates from '../util/get-duplicates'; @@ -94,11 +95,19 @@ export function merge( ); } + // Explicit mappings win. Otherwise, match source to target workflows + // by uuid, then id, then name + const workflowMappings = noMappings + ? matchWorkflows(sourceWorkflows, target.workflows) + : options.workflowMappings; + const getTargetWorkflow = (sourceWorkflow: Workflow) => { + const targetId = workflowMappings[sourceWorkflow.id]; + return targetId ? target.getWorkflow(targetId) : undefined; + }; + const potentialConflicts: Record = {}; for (const sourceWorkflow of sourceWorkflows) { - const targetId = - options.workflowMappings?.[sourceWorkflow.id] ?? sourceWorkflow.id; - const targetWorkflow = target.getWorkflow(targetId); + const targetWorkflow = getTargetWorkflow(sourceWorkflow); if (targetWorkflow && !sourceWorkflow.canMergeInto(targetWorkflow)) { potentialConflicts[sourceWorkflow.id] = targetWorkflow?.id; } @@ -115,9 +124,7 @@ export function merge( } for (const sourceWorkflow of sourceWorkflows) { - const targetId = - options.workflowMappings?.[sourceWorkflow.id] ?? sourceWorkflow.id; - const targetWorkflow = target.getWorkflow(targetId); + const targetWorkflow = getTargetWorkflow(sourceWorkflow); if (targetWorkflow) { usedTargetIds.add(targetWorkflow.id); diff --git a/packages/project/src/parse/from-fs.ts b/packages/project/src/parse/from-fs.ts index 4772f0577..6cd8c7ecc 100644 --- a/packages/project/src/parse/from-fs.ts +++ b/packages/project/src/parse/from-fs.ts @@ -1,6 +1,6 @@ import fs from 'node:fs/promises'; import path from 'node:path'; -import { glob } from 'glob'; +import { escape, glob } from 'glob'; import * as l from '@openfn/lexicon'; import { Project } from '../Project'; @@ -12,7 +12,7 @@ import { findWorkspaceFile, loadCheckoutFile, } from '../util/config'; -import { countBy, omit } from 'lodash-es'; +import { countBy, isPlainObject, omit } from 'lodash-es'; import { Logger } from '@openfn/logger'; import omitNil from '../util/omit-nil'; import slugify from '../util/slugify'; @@ -33,8 +33,8 @@ export type FromFsConfig = { }; // Parse a single project from a root folder -// Note that this does NOT attempt to load UUIDS from the project file -// It just builds the project on disk +// Workflow uuids are not stored in the workflow files: if a workflow is tracked +// in the checkout file, its uuid is loaded from there. Other uuids are not loaded // I suppose we could take an option? export const parseProject = async (options: FromFsConfig) => { const { root, logger, alias } = options; @@ -45,10 +45,20 @@ export const parseProject = async (options: FromFsConfig) => { const checkout = loadCheckoutFile(root, options.branch, context.project) ?? {}; + const workflowUuids = isPlainObject(checkout.workflow_uuids) + ? (checkout.workflow_uuids as Record) + : undefined; + const proj: any = { id: options.name ? slugify(options.name) : checkout.id, name: options.name ? slugify(options.name) : checkout.name, - openfn: omit(checkout, ['id', 'alias', 'forked_from', 'collections']), + openfn: omit(checkout, [ + 'id', + 'alias', + 'forked_from', + 'workflow_uuids', + 'collections', + ]), // Legacy: collections used to be a list of names in openfn.yaml. A // collections key in resources.yaml takes precedence (see below) @@ -59,6 +69,7 @@ export const parseProject = async (options: FromFsConfig) => { workflows: [], cli: omitNil({ forked_from: checkout.forked_from, + workflow_uuids: workflowUuids, }), }; @@ -86,7 +97,7 @@ export const parseProject = async (options: FromFsConfig) => { const workflowDir = (config as any).workflowRoot ?? config.dirs?.workflows ?? 'workflows'; const fileType = config.formats?.workflow ?? 'yaml'; - const pattern = path.resolve(root, workflowDir) + `/**/*.${fileType}`; + const pattern = escape(path.resolve(root, workflowDir)) + `/**/*.${fileType}`; const candidateWfs = await glob(pattern, { ignore: ['**node_modules/**', '**tmp**'], }); @@ -112,8 +123,9 @@ export const parseProject = async (options: FromFsConfig) => { wf = wf.workflow; } - if (wf.id && Array.isArray(wf.steps)) { - //logger?.log('Loading workflow at ', filePath); // TODO logger.debug + if (Array.isArray(wf.steps)) { + wf.id = path.basename(filePath, path.extname(filePath)); + for (const step of wf.steps) { // This is the saved, remote view of the step // TODO if the id has changed, how do we track? @@ -139,6 +151,11 @@ export const parseProject = async (options: FromFsConfig) => { } const workflow = new Workflow(wf); + // If the workflow is tracked, we know its uuid + const uuid = workflowUuids?.[workflow.id]; + if (uuid) { + workflow.openfn = { ...workflow.openfn, uuid }; + } // A workflow folder renamed away from the id is treated as an alias const dir = path.dirname(filePath); const folder = path.basename(dir); diff --git a/packages/project/src/serialize/to-fs.ts b/packages/project/src/serialize/to-fs.ts index 536472ef5..a73140852 100644 --- a/packages/project/src/serialize/to-fs.ts +++ b/packages/project/src/serialize/to-fs.ts @@ -69,6 +69,9 @@ export const extractWorkflow = ( const wf = Object.assign( { + // DEPRECATED: the id is a local artefact and is ignored on load (the file name is used) + // It's still written so that older CLI versions can read these files + // TODO remove this in the next major id: workflow.id, name: workflow.name, start: workflow.start, diff --git a/packages/project/src/util/config.ts b/packages/project/src/util/config.ts index b4801e834..d2a9f1c97 100644 --- a/packages/project/src/util/config.ts +++ b/packages/project/src/util/config.ts @@ -71,6 +71,18 @@ export const extractCheckout = ( project.forked_from = source.cli.forked_from; } + // Track the uuid of each workflow, by local id + // This is recorded for future use and isn't read by anything yet + const workflowUuids = source.workflows.reduce((obj: any, wf) => { + if (wf.openfn?.uuid) { + obj[wf.id] = wf.openfn.uuid; + } + return obj; + }, {}); + if (Object.keys(workflowUuids).length) { + project.workflow_uuids = workflowUuids; + } + return { path: getCheckoutPath(branch), content: jsonToYaml(sortKeys(project)), diff --git a/packages/project/src/util/project-diff.ts b/packages/project/src/util/project-diff.ts index f942bfad3..2b71b524a 100644 --- a/packages/project/src/util/project-diff.ts +++ b/packages/project/src/util/project-diff.ts @@ -1,12 +1,16 @@ import { isEqual } from 'lodash-es'; import { Project } from '../Project'; import { toResourceChannels } from './resources'; +import matchWorkflows from '../merge/match-workflows'; export type DiffType = 'added' | 'changed' | 'removed'; export type WorkflowDiff = { + // The workflow id in A (or in B, if the workflow was added) id: string; type: DiffType; + // The workflow id in B, if it differs from the id in A (ie, it was renamed) + bId?: string; }; export type ChannelDiff = { @@ -34,7 +38,9 @@ export type ProjectDiff = { * Compare two projects and return the changes showing how * project B has diverged from project A. * - * Workflows are identified by their ID and compared using version hashes. + * Workflows are paired up by uuid, then id, then name (see matchWorkflows) and + * compared using version hashes. A workflow with a different id in each project + * is reported with its id in A, and its id in B as `bId`. * * @param a - The baseline project (e.g., main branch) * @param b - The comparison project (e.g., staging branch) @@ -48,6 +54,9 @@ export type ProjectDiff = { * - 'removed': workflow exists in A but not in B * - 'changed': workflow exists in both but has different version hashes * + * If a workflow list is passed, a workflow is only considered if the list + * includes its id in A or in B. + * * @example * ```typescript * const main = await Project.from('fs', { root: '.' }); @@ -64,30 +73,39 @@ export function diff( ): ProjectDiff { const diffs: WorkflowDiff[] = []; + const inScope = (...ids: Array) => + !workflows?.length || ids.some((id) => id && workflows.includes(id)); + + // B's workflow id -> A's workflow id + const matches = matchWorkflows(b.workflows, a.workflows); + const matchedInB = Object.fromEntries( + Object.entries(matches).map(([idB, idA]) => [idA, idB]) + ); + // Check all of project A's workflows for (const workflowA of a.workflows) { - if (workflows?.length && !workflows.includes(workflowA.id)) { + const workflowB = b.getWorkflow(matchedInB[workflowA.id]); + + if (!inScope(workflowA.id, workflowB?.id)) { continue; } - const workflowB = b.getWorkflow(workflowA.id); - if (!workflowB) { // workflow exists in A but not in B = removed diffs.push({ id: workflowA.id, type: 'removed' }); } else if (workflowA.getVersionHash() !== workflowB.getVersionHash()) { // workflow exists in both but with different content = changed - diffs.push({ id: workflowA.id, type: 'changed' }); + diffs.push({ + id: workflowA.id, + type: 'changed', + ...(workflowB.id !== workflowA.id && { bId: workflowB.id }), + }); } } // Check for workflows that were added in B for (const workflowB of b.workflows) { - if (workflows?.length && !workflows.includes(workflowB.id)) { - continue; - } - - if (!a.getWorkflow(workflowB.id)) { + if (!(workflowB.id in matches) && inScope(workflowB.id)) { // workflow exists in B but not in A = added diffs.push({ id: workflowB.id, type: 'added' }); } diff --git a/packages/project/src/util/relabel-workflows.ts b/packages/project/src/util/relabel-workflows.ts new file mode 100644 index 000000000..630e46286 --- /dev/null +++ b/packages/project/src/util/relabel-workflows.ts @@ -0,0 +1,24 @@ +import Project from '../Project'; +import Workflow from '../Workflow'; + +/** + * Returns a copy of the project with some workflows given new ids + * + * `ids` maps current workflow id -> new id. Other workflows are untouched. + * + * The copy is shallow: only the workflow list and the cli meta are new + */ +export default function relabelWorkflows( + project: Project, + ids: Record +): Project { + const workflows = project.workflows.map((wf) => + ids[wf.id] && ids[wf.id] !== wf.id + ? new Workflow({ ...(wf.toJSON() as any), id: ids[wf.id] }) + : wf + ); + return Object.assign(Object.create(Object.getPrototypeOf(project)), project, { + workflows, + cli: { ...project.cli }, + }); +} diff --git a/packages/project/test/merge/match-workflows.test.ts b/packages/project/test/merge/match-workflows.test.ts new file mode 100644 index 000000000..b00ef7c53 --- /dev/null +++ b/packages/project/test/merge/match-workflows.test.ts @@ -0,0 +1,88 @@ +import test from 'ava'; +import Workflow from '../../src/Workflow'; +import matchWorkflows from '../../src/merge/match-workflows'; + +const wf = (id: string, name?: string, uuid?: string) => + new Workflow({ + id, + name, + steps: [], + ...(uuid && { openfn: { uuid } }), + } as any); + +test('match by id', (t) => { + const result = matchWorkflows([wf('a'), wf('b')], [wf('b'), wf('c')]); + t.deepEqual(result, { b: 'b' }); +}); + +test('match by uuid when the ids differ (a rename)', (t) => { + const result = matchWorkflows( + [wf('new-id', 'Wf', 'uuid-1')], + [wf('old-id', 'Wf', 'uuid-1')] + ); + t.deepEqual(result, { 'new-id': 'old-id' }); +}); + +test('match by name when there is no uuid and the ids differ', (t) => { + const result = matchWorkflows( + [wf('a', 'My Workflow')], + [wf('b', 'My Workflow')] + ); + t.deepEqual(result, { a: 'b' }); +}); + +test('uuid beats id', (t) => { + // each source shares an id with one target, and a uuid with the other + const result = matchWorkflows( + [wf('a', 'A', 'uuid-1'), wf('b', 'B', 'uuid-2')], + [wf('a', 'A', 'uuid-2'), wf('b', 'B', 'uuid-1')] + ); + t.deepEqual(result, { a: 'b', b: 'a' }); +}); + +test('id beats name', (t) => { + const result = matchWorkflows( + [wf('a', 'Same'), wf('b', 'Other')], + [wf('b', 'Same'), wf('c', 'Other')] + ); + // a has the name of target b, but b is already claimed by id + t.deepEqual(result, { b: 'b' }); +}); + +test('ids match even if the uuids differ (eg, merging a sandbox)', (t) => { + const result = matchWorkflows( + [wf('a', 'A', 'uuid-1')], + [wf('a', 'A', 'uuid-2')] + ); + t.deepEqual(result, { a: 'a' }); +}); + +test('matches are one-to-one', (t) => { + // both sources want the same target + const result = matchWorkflows( + [wf('a', 'A', 'uuid-1'), wf('b', 'B', 'uuid-1')], + [wf('x', 'X', 'uuid-1')] + ); + t.deepEqual(result, { a: 'x' }); +}); + +test('do not match by name if the name is not unique in the source', (t) => { + const result = matchWorkflows( + [wf('a', 'Same'), wf('b', 'Same')], + [wf('x', 'Same')] + ); + t.deepEqual(result, {}); +}); + +test('do not match by name if the name is not unique in the target', (t) => { + const result = matchWorkflows( + [wf('a', 'Same')], + [wf('x', 'Same'), wf('y', 'Same')] + ); + t.deepEqual(result, {}); +}); + +test('workflows with no name are never matched by name', (t) => { + const result = matchWorkflows([wf('a')], [wf('b')]); + t.deepEqual(result, {}); +}); diff --git a/packages/project/test/merge/merge-project.test.ts b/packages/project/test/merge/merge-project.test.ts index 1a92f3ce0..8671afeb1 100644 --- a/packages/project/test/merge/merge-project.test.ts +++ b/packages/project/test/merge/merge-project.test.ts @@ -1097,8 +1097,8 @@ test('id match: same workflow in source and target project', (t) => { }); test('no id match: union of both workflow', (t) => { - const source = generateWorkflow('@id workflow a-b'); - const target = generateWorkflow('@id another_workflow a-b'); + const source = generateWorkflow('@id workflow @name one a-b'); + const target = generateWorkflow('@id another_workflow @name two a-b'); const source_project = createProject(source); const target_project = createProject(target); @@ -1167,16 +1167,67 @@ test('id match: preserve target uuid', (t) => { t.is(result.workflows[0].openfn.uuid, target.openfn.uuid); }); +test('uuid match: a renamed workflow keeps the source id and the target uuid', (t) => { + const source = generateWorkflow('@id local-id @name Wf a-new', { + openfnUuid: true, + }); + const target = generateWorkflow('@id remote-id @name Wf a-old', { + openfnUuid: true, + }); + // the workflows are linked by uuid + source.openfn = { uuid: 'wf-uuid' }; + target.openfn = { uuid: 'wf-uuid' }; + + const result = merge(createProject(source), createProject(target)); + + t.is(result.workflows.length, 1); + t.is(result.workflows[0].id, 'local-id'); + t.is(result.workflows[0].openfn?.uuid, 'wf-uuid'); + t.truthy(result.workflows[0].get('new')); +}); + +test('name match: a workflow with a different id but the same name is merged', (t) => { + const source = generateWorkflow('@id local-id @name Wf a-new'); + const target = generateWorkflow('@id remote-id @name Wf a-old'); + + const result = merge(createProject(source), createProject(target)); + + t.is(result.workflows.length, 1); + t.is(result.workflows[0].id, 'local-id'); + t.truthy(result.workflows[0].get('new')); +}); + +test('explicit workflow mappings are not overridden by uuid or name matching', (t) => { + const source = generateWorkflow('@id x @name Same a-new'); + const target1 = generateWorkflow('@id y @name Same a-old'); + const target2 = generateWorkflow('@id z @name Other a-other'); + source.openfn = { uuid: 'uuid-1' }; + target1.openfn = { uuid: 'uuid-1' }; + + const result = merge( + createProject(source), + createProject([target1, target2]), + { + workflowMappings: { x: 'z' }, + } + ); + + // x was merged into z, and y was left alone + t.truthy(result.getWorkflow('x')?.get('new')); + t.truthy(result.getWorkflow('y')?.get('old')); + t.falsy(result.getWorkflow('z')); +}); + test('options: no mappings & removeUnmapped=false', (t) => { const source_project = createProject([ generateWorkflow('@id a a-new'), generateWorkflow('@id b b-new'), - generateWorkflow('@id c c-new'), + generateWorkflow('@id c @name unique-c c-new'), ]); const target_project = createProject([ generateWorkflow('@id a a-old'), generateWorkflow('@id b b-old'), - generateWorkflow('@id d d-old'), + generateWorkflow('@id d @name unique-d d-old'), ]); const result = merge(source_project, target_project); // should be the new version of a,b,c diff --git a/packages/project/test/parse/from-fs.test.ts b/packages/project/test/parse/from-fs.test.ts index 22dc5d4a2..3d16c47f2 100644 --- a/packages/project/test/parse/from-fs.test.ts +++ b/packages/project/test/parse/from-fs.test.ts @@ -67,6 +67,29 @@ test.serial('should include multiple workflows (legacy format)', async (t) => { t.is(wf2.name, 'Workflow 2'); }); +test.serial( + 'should treat square brackets in the project root literally', + async (t) => { + // an unescaped [v1] would also match the sibling folder /ws/projv + mockFile('/ws/proj[v1]/openfn.yaml', buildConfig()); + mockFile('/ws/proj[v1]/workflows/mine/mine.yaml', { + id: 'mine', + steps: [], + }); + mockFile('/ws/projv/workflows/other/other.yaml', { + id: 'other', + steps: [], + }); + + const project = await parseProject({ root: '/ws/proj[v1]' }); + + t.deepEqual( + project.workflows.map((wf) => wf.id), + ['mine'] + ); + } +); + test.serial('should treat a renamed workflow folder as an alias', async (t) => { mockFile('/ws/openfn.yaml', buildConfig()); @@ -221,6 +244,80 @@ test.serial('should track forked_from', async (t) => { t.falsy(project.openfn!.forked_from); }); +const checkoutWithWorkflow = (checkout: object) => { + mockFile('/ws/openfn.yaml', buildConfig()); + mockFile('/ws/.openfn/checkout.yaml', { id: 'my-project', ...checkout }); + mockFile('/ws/workflows/workflow-1/workflow-1.yaml', { + name: 'Workflow 1', + steps: [{ id: 'a' }], + }); +}; + +test.serial('should track workflow_uuids from the checkout file', async (t) => { + checkoutWithWorkflow({ workflow_uuids: { 'workflow-1': 'uuid-1' } }); + + const project = await parseProject({ root: '/ws' }); + + t.deepEqual(project.cli.workflow_uuids, { 'workflow-1': 'uuid-1' }); + // it's tracking metadata, not project config + t.falsy(project.openfn!.workflow_uuids); +}); + +test.serial('should set the uuid on a tracked workflow', async (t) => { + checkoutWithWorkflow({ workflow_uuids: { 'workflow-1': 'uuid-1' } }); + mockFile('/ws/workflows/workflow-2/workflow-2.yaml', { + name: 'Workflow 2', + steps: [{ id: 'a' }], + }); + + const project = await parseProject({ root: '/ws' }); + + t.is(project.getWorkflow('workflow-1')!.openfn?.uuid, 'uuid-1'); + // untracked workflows have no uuid + t.falsy(project.getWorkflow('workflow-2')!.openfn?.uuid); +}); + +test.serial( + 'should find the uuid of a tracked workflow in a renamed folder', + async (t) => { + mockFile('/ws/openfn.yaml', buildConfig()); + mockFile('/ws/.openfn/checkout.yaml', { + id: 'my-project', + workflow_uuids: { 'workflow-1': 'uuid-1' }, + }); + mockFile('/ws/workflows/my-folder/workflow-1.yaml', { + name: 'Workflow 1', + steps: [{ id: 'a' }], + }); + + const project = await parseProject({ root: '/ws' }); + + const wf = project.getWorkflow('workflow-1')!; + t.is(wf.alias, 'my-folder'); + t.is(wf.openfn?.uuid, 'uuid-1'); + } +); + +test.serial('should load a checkout file with no workflow_uuids', async (t) => { + checkoutWithWorkflow({ forked_from: { 'workflow-1': 'abcd' } }); + + const project = await parseProject({ root: '/ws' }); + + t.is(project.workflows.length, 1); + t.is(project.cli.workflow_uuids, undefined); + t.deepEqual(project.cli.forked_from, { 'workflow-1': 'abcd' }); +}); + +test.serial('should ignore a malformed workflow_uuids', async (t) => { + checkoutWithWorkflow({ workflow_uuids: 'nope' }); + + const project = await parseProject({ root: '/ws' }); + + t.is(project.workflows.length, 1); + t.is(project.cli.workflow_uuids, undefined); + t.falsy(project.openfn!.workflow_uuids); +}); + test.serial('should load the name from config', async (t) => { mockFile('/ws/openfn.yaml', { workspace: buildConfig(), @@ -436,3 +533,35 @@ test.serial('should override the name and id from options', async (t) => { t.is(project.name, 'hello-world'); t.is(project.id, 'hello-world'); }); + +test.serial('should take the workflow id from the file name', async (t) => { + mockFile('/ws/openfn.yaml', buildConfig()); + + // no id in the file, and a name that would slugify to something else + mockFile('/ws/workflows/my-workflow/my-workflow.yaml', { + name: 'My Workflow (renamed)', + steps: [{ id: 'a' }], + }); + + const project = await parseProject({ root: '/ws' }); + + t.is(project.workflows.length, 1); + const wf = project.getWorkflow('my-workflow')!; + t.is(wf.name, 'My Workflow (renamed)'); + t.is(wf.alias, undefined); +}); + +test.serial('should prefer the file name to an id in the file', async (t) => { + mockFile('/ws/openfn.yaml', buildConfig()); + + mockFile('/ws/workflows/my-workflow/my-workflow.yaml', { + id: 'something-else', + name: 'My Workflow', + steps: [{ id: 'a' }], + }); + + const project = await parseProject({ root: '/ws' }); + + t.is(project.workflows.length, 1); + t.is(project.workflows[0].id, 'my-workflow'); +}); diff --git a/packages/project/test/util/config.test.ts b/packages/project/test/util/config.test.ts index 0cd128af3..63434d79e 100644 --- a/packages/project/test/util/config.test.ts +++ b/packages/project/test/util/config.test.ts @@ -8,6 +8,7 @@ import { loadWorkspaceFile, } from '../../src/util/config'; import mock from 'mock-fs'; +import { yamlToJson } from '../../src/util/yaml'; import Project from '../../src'; test.afterEach(() => { @@ -330,3 +331,41 @@ test('hasCheckoutMeta: looks for the checkout file for the given branch', (t) => t.false(hasCheckoutMeta('/ws', 'main')); t.false(hasCheckoutMeta('/ws')); }); + +test('generate checkout file with workflow_uuids, keyed by workflow id', (t) => { + const proj = new Project({ + id: 'my-project', + workflows: [ + { id: 'wf-a', name: 'A', steps: [], openfn: { uuid: 'uuid-a' } }, + { id: 'wf-b', name: 'B', steps: [], openfn: { uuid: 'uuid-b' } }, + ], + }); + const result = extractCheckout(proj); + t.deepEqual(yamlToJson(result.content).workflow_uuids, { + 'wf-a': 'uuid-a', + 'wf-b': 'uuid-b', + }); +}); + +test('checkout file workflow_uuids skips workflows with no uuid', (t) => { + const proj = new Project({ + id: 'my-project', + workflows: [ + { id: 'wf-a', name: 'A', steps: [], openfn: { uuid: 'uuid-a' } }, + { id: 'wf-b', name: 'B', steps: [] }, + ], + }); + const result = extractCheckout(proj); + t.deepEqual(yamlToJson(result.content).workflow_uuids, { + 'wf-a': 'uuid-a', + }); +}); + +test("checkout file excludes workflow_uuids if there aren't any", (t) => { + const proj = new Project({ + id: 'my-project', + workflows: [{ id: 'wf-b', name: 'B', steps: [] }], + }); + const result = extractCheckout(proj); + t.false(result.content.includes('workflow_uuids')); +}); diff --git a/packages/project/test/util/project-diff.test.ts b/packages/project/test/util/project-diff.test.ts index 509527761..171157bb9 100644 --- a/packages/project/test/util/project-diff.test.ts +++ b/packages/project/test/util/project-diff.test.ts @@ -150,8 +150,8 @@ test('diff: should only consider removed workflows from a filter list', (t) => { test('diff: should detect multiple changes at once', (t) => { const wf1 = generateWorkflow('@id a trigger-x'); const wf2 = generateWorkflow('@id b trigger-y'); - const wf3 = generateWorkflow('@id c trigger-z'); - const wf4 = generateWorkflow('@id d trigger-w'); + const wf3 = generateWorkflow('@id c @name wf-c trigger-z'); + const wf4 = generateWorkflow('@id d @name wf-d trigger-w'); // wf2 will be changed in projectB const wf2Changed = generateWorkflow('@id b trigger-different'); @@ -242,6 +242,74 @@ const channel = (name: string, extra = {}) => ({ const withChannels = (channels?: any[]) => new Project({ name: 'p', workflows: [], channels }); +const withUuid = (wf: any, uuid: string) => { + wf.openfn = { uuid }; + return wf; +}; + +test('diff: a workflow with a different id but the same uuid is changed, not added and removed', (t) => { + const wfA = withUuid(generateWorkflow('@id old-id @name Wf trigger-x'), 'u1'); + const wfB = withUuid(generateWorkflow('@id new-id @name Wf trigger-y'), 'u1'); + + const projectA = new Project({ name: 'a', workflows: [wfA] }); + const projectB = new Project({ name: 'b', workflows: [wfB] }); + + const { workflows: diffs } = diff(projectA, projectB); + + t.deepEqual(diffs, [{ id: 'old-id', type: 'changed', bId: 'new-id' }]); +}); + +test('diff: a workflow with a different id but the same name is changed', (t) => { + const wfA = generateWorkflow('@id old-id @name Wf trigger-x'); + const wfB = generateWorkflow('@id new-id @name Wf trigger-y'); + + const projectA = new Project({ name: 'a', workflows: [wfA] }); + const projectB = new Project({ name: 'b', workflows: [wfB] }); + + const { workflows: diffs } = diff(projectA, projectB); + + t.deepEqual(diffs, [{ id: 'old-id', type: 'changed', bId: 'new-id' }]); +}); + +test('diff: a workflow with only a different id and identical content has no diff', (t) => { + const wfA = withUuid(generateWorkflow('@id old-id @name Wf trigger-x'), 'u1'); + const wfB = withUuid(generateWorkflow('@id new-id @name Wf trigger-x'), 'u1'); + + const projectA = new Project({ name: 'a', workflows: [wfA] }); + const projectB = new Project({ name: 'b', workflows: [wfB] }); + + const { workflows: diffs } = diff(projectA, projectB); + + t.is(diffs.length, 0); +}); + +test('diff: a renamed workflow is considered if the filter list has its id in either project', (t) => { + const wfA = withUuid(generateWorkflow('@id old-id @name Wf trigger-x'), 'u1'); + const wfB = withUuid(generateWorkflow('@id new-id @name Wf trigger-y'), 'u1'); + + const projectA = new Project({ name: 'a', workflows: [wfA] }); + const projectB = new Project({ name: 'b', workflows: [wfB] }); + + t.is(diff(projectA, projectB, ['new-id']).workflows.length, 1); + t.is(diff(projectA, projectB, ['old-id']).workflows.length, 1); + t.is(diff(projectA, projectB, ['other']).workflows.length, 0); +}); + +test('diff: workflows with no uuid, id or name in common are still added and removed', (t) => { + const wfA = generateWorkflow('@id a @name wf-a trigger-x'); + const wfB = generateWorkflow('@id b @name wf-b trigger-x'); + + const projectA = new Project({ name: 'a', workflows: [wfA] }); + const projectB = new Project({ name: 'b', workflows: [wfB] }); + + const { workflows: diffs } = diff(projectA, projectB); + + t.deepEqual(diffs, [ + { id: 'a', type: 'removed' }, + { id: 'b', type: 'added' }, + ]); +}); + test('diff: resources.channels is empty for identical channels', (t) => { const a = withChannels([channel('one')]); const b = withChannels([channel('one')]); diff --git a/packages/project/test/util/relabel-workflows.test.ts b/packages/project/test/util/relabel-workflows.test.ts new file mode 100644 index 000000000..9ac6c355c --- /dev/null +++ b/packages/project/test/util/relabel-workflows.test.ts @@ -0,0 +1,56 @@ +import test from 'ava'; +import Project from '../../src'; +import relabelWorkflows from '../../src/util/relabel-workflows'; + +const createProject = () => + new Project({ + id: 'p', + workflows: [ + { + id: 'wf-a', + name: 'A', + steps: [{ id: 'step', expression: 'fn(s => s)' }], + history: ['abcd'], + openfn: { uuid: 'uuid-a' }, + }, + { id: 'wf-b', name: 'B', steps: [], openfn: { uuid: 'uuid-b' } }, + ], + } as any); + +test('give a workflow a new id, keeping everything else', (t) => { + const result = relabelWorkflows(createProject(), { 'wf-a': 'renamed' }); + + t.deepEqual( + result.workflows.map((w) => w.id), + ['renamed', 'wf-b'] + ); + const wf = result.getWorkflow('renamed')!; + t.is(wf.name, 'A'); + t.is(wf.openfn?.uuid, 'uuid-a'); + t.deepEqual(wf.history, ['abcd']); + t.is(wf.steps[0].expression, 'fn(s => s)'); +}); + +test('does not mutate the original project', (t) => { + const project = createProject(); + relabelWorkflows(project, { 'wf-a': 'renamed' }); + + t.truthy(project.getWorkflow('wf-a')); + t.falsy(project.getWorkflow('renamed')); +}); + +test('ids that are not mapped are left alone', (t) => { + const result = relabelWorkflows(createProject(), { nope: 'x' }); + t.deepEqual( + result.workflows.map((w) => w.id), + ['wf-a', 'wf-b'] + ); +}); + +test('does not share cli meta with the original project', (t) => { + const project = createProject(); + const result = relabelWorkflows(project, { 'wf-a': 'renamed' }); + result.cli.forked_from = { renamed: 'abcd' }; + + t.is(project.cli.forked_from, undefined); +}); diff --git a/packages/ws-worker/CHANGELOG.md b/packages/ws-worker/CHANGELOG.md index 2b29ae430..f3e1886fe 100644 --- a/packages/ws-worker/CHANGELOG.md +++ b/packages/ws-worker/CHANGELOG.md @@ -1,5 +1,11 @@ # ws-worker +## 1.30.0 + +### Minor Changes + +- aaaed2e: Add an `immediate` option to worker destroy to tear down without waiting for runs + ## 1.29.6 ### Patch Changes diff --git a/packages/ws-worker/package.json b/packages/ws-worker/package.json index 08cde663d..3739025d6 100644 --- a/packages/ws-worker/package.json +++ b/packages/ws-worker/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/ws-worker", - "version": "1.29.6", + "version": "1.30.0", "description": "A Websocket Worker to connect Lightning to a Runtime Engine", "main": "dist/index.js", "type": "module", diff --git a/packages/ws-worker/src/api/destroy.ts b/packages/ws-worker/src/api/destroy.ts index a4a521c0e..bb9e0f867 100644 --- a/packages/ws-worker/src/api/destroy.ts +++ b/packages/ws-worker/src/api/destroy.ts @@ -3,8 +3,20 @@ import { INTERNAL_CLAIM_COMPLETE, INTERNAL_RUN_COMPLETE } from '../events'; import type { Logger } from '@openfn/logger'; -const destroy = async (app: ServerApp, logger: Logger) => { - logger.info('Closing server...'); +export type DestroyOptions = { + // Tear everything down now: don't wait for active runs or claims to complete + immediate?: boolean; +}; + +const destroy = async ( + app: ServerApp, + logger: Logger, + options: DestroyOptions = {} +) => { + const { immediate = false } = options; + logger.info( + immediate ? 'Closing server immediately...' : 'Closing server...' + ); // Close the server AND wait for runs to complete in parallel // The http server can take a moment to close and the two @@ -22,14 +34,21 @@ const destroy = async (app: ServerApp, logger: Logger) => { app.server.close(async () => { resolve(); }); + + if (immediate) { + // Don't wait for open connections to drain + app.server.closeAllConnections?.(); + } }), (async () => { - // Let any active runs complete - await waitForRunsAndClaims(app, logger); + if (!immediate) { + // Let any active runs complete + await waitForRunsAndClaims(app, logger); + } app.queueChannel?.leave(); // Kill the engine and socket - await app.engine.destroy(); + await app.engine.destroy(immediate); app.socket?.disconnect(); })(), ]); diff --git a/packages/ws-worker/src/server.ts b/packages/ws-worker/src/server.ts index 3c95d2f0b..47fa04dba 100644 --- a/packages/ws-worker/src/server.ts +++ b/packages/ws-worker/src/server.ts @@ -17,6 +17,7 @@ import { WORK_AVAILABLE, } from './events'; import destroy from './api/destroy'; +import type { DestroyOptions } from './api/destroy'; import { Workloop } from './api/workloop'; import claim from './api/claim'; import { Context, execute } from './api/execute'; @@ -87,7 +88,7 @@ export interface ServerApp extends Koa { runWorkloopMap: Record; execute: ({ id, token }: ClaimRun) => Promise; - destroy: () => void; + destroy: (options?: DestroyOptions) => Promise; resumeWorkloop: (workloop?: Workloop) => void; pendingClaims: () => number; @@ -469,7 +470,7 @@ function createServer(engine: RuntimeEngine, options: ServerOptions = {}) { app.pendingClaims = () => app.workloops.reduce((sum, w) => sum + Object.keys(w.openClaims).length, 0); - app.destroy = () => destroy(app, logger); + app.destroy = (options?: DestroyOptions) => destroy(app, logger, options); app.use(router.routes()); diff --git a/packages/ws-worker/test/api/destroy.test.ts b/packages/ws-worker/test/api/destroy.test.ts index 748f1e8fc..9d7c81c6b 100644 --- a/packages/ws-worker/test/api/destroy.test.ts +++ b/packages/ws-worker/test/api/destroy.test.ts @@ -337,3 +337,72 @@ test("don't claim after destroy", async (t) => { lightning.enqueueRun(createRun()); }); }); + +test.serial( + 'immediate destroy does not wait for an outstanding claim', + async (t) => { + // Delay the claim response so it is still outstanding when destroy returns + initLightning({ socketDelay: 200 }); + await initWorker({ noLoop: true }); + + return new Promise((done) => { + const doDestroy = async () => { + t.is(worker.pendingClaims(), 1); + + await destroy(worker, logger, { immediate: true }); + + // A graceful destroy would block until the claim completed + t.is(worker.pendingClaims(), 1); + t.true(worker.destroyed); + t.false(await ping()); + + done(); + }; + + worker.events.once(INTERNAL_CLAIM_START, () => { + doDestroy(); + }); + + lightning.enqueueRun(createRun()); + worker.claim().catch(() => {}); + }); + } +); + +test.serial( + 'immediate destroy tells the engine to destroy instantly', + async (t) => { + initLightning(); + await initWorker(); + + let instant: boolean | undefined; + const originalDestroy = worker.engine.destroy; + worker.engine.destroy = async (arg?: boolean) => { + instant = arg; + return originalDestroy(arg); + }; + + await worker.destroy({ immediate: true }); + + t.true(instant); + } +); + +test.serial( + 'a normal destroy does not ask the engine to destroy instantly', + async (t) => { + initLightning(); + await initWorker(); + + let instant: boolean | undefined = true; + const originalDestroy = worker.engine.destroy; + worker.engine.destroy = async (arg?: boolean) => { + instant = arg; + return originalDestroy(arg); + }; + + await worker.destroy(); + + t.falsy(instant); + } +);