diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml index 51fb4aa..4a33084 100644 --- a/.github/workflows/verify.yml +++ b/.github/workflows/verify.yml @@ -32,12 +32,22 @@ jobs: - name: Set up Docker Buildx uses: docker/setup-buildx-action@v4 - name: Build container + id: build uses: docker/build-push-action@v7 env: DOCKER_BUILD_RECORD_UPLOAD: 'false' with: context: ./ load: true - tags: steadybit/cli:latest + # By image id rather than a tag. `docker run` pulls a tag it cannot find locally, + # so a name like steadybit/cli:latest would quietly fall back to the last release + # if this build ever stopped being loaded. An id is never looked up in a registry. - name: Test container - run: docker run --rm steadybit/cli:latest -V + run: docker run --rm ${{ steps.build.outputs.imageid }} -V + # Everything these cover needs a real process: an exit status, a terminal, or the + # spawn of a subcommand. They run against the image users install, built from the + # npm tarball, so the packaging is exercised too. + - name: Run container smoke tests + run: | + docker run --rm -v "$PWD/e2e:/e2e" --entrypoint sh \ + ${{ steps.build.outputs.imageid }} /e2e/run.sh diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b4e72d9..edeadcd 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -13,6 +13,45 @@ npm ci Run `npm run ci` before pushing. It type-checks, tests, lints and builds, and is the same script CI runs. +## Tests + +Tests sit at three levels. Put a test at the lowest one that can hold it — the +levels get slower and harder to debug as you go down this list. + +| Level | Tool | Covers | +| --------- | ---------------------------------- | ------------------------------------------------------ | +| Unit | vitest | A single function or class, no I/O | +| Command | vitest + msw + `@inquirer/testing` | A command end to end in process, including its prompts | +| Container | `e2e/run.sh` + expect | Only what needs a real process | + +Prompts are driven through `@inquirer/testing`. Mock the prompt package with +`wrapPrompt` so the application's own call is intercepted, and use the helpers in +`src/mocks/prompts.ts` rather than writing to the screen directly: + +```ts +vi.mock('@inquirer/input', async importOriginal => { + const actual = await importOriginal(); + return { ...actual, default: wrapPrompt(actual.default) }; +}); + +await answerPrompt('Profile name:', 'my-profile'); +``` + +The container tests are deliberately thin. They exist for the four things no in-process +test can reach — real exit codes, a real terminal, the spawn of a subcommand, and +the packaged artifact — and they assert exit status and a line of output, never +content. Anything checking structure belongs at the command level. + +```sh +docker build -t steadybit/cli:under-test . +docker run --rm -v "$PWD/e2e:/e2e" --entrypoint sh steadybit/cli:under-test /e2e/run.sh +``` + +The scripts are mounted into the image rather than baked into a derived one, and CI runs +the image by id rather than by name. Both avoid the same mistake: a name is resolved +against a registry when it cannot be found locally, so the suite can end up exercising +the last release while reporting success. + ### Local CLI Execution ```sh diff --git a/e2e/add-profile.exp b/e2e/add-profile.exp new file mode 100644 index 0000000..9557565 --- /dev/null +++ b/e2e/add-profile.exp @@ -0,0 +1,10 @@ +# Drives `config profile add` through a real pty. +log_user 0 +set timeout 20 +spawn steadybit config profile add +expect "Profile name:" { send "e2e\r" } +expect "Base URL" { send "https://platform.example.com\r" } +expect "API access token:" { send "s3cr3t\r" } +expect eof +catch wait result +exit [lindex $result 3] diff --git a/e2e/cancel-profile.exp b/e2e/cancel-profile.exp new file mode 100644 index 0000000..2f5b2e3 --- /dev/null +++ b/e2e/cancel-profile.exp @@ -0,0 +1,9 @@ +# Ctrl+C at the first question must exit quietly with the SIGINT status. +log_user 0 +set timeout 20 +spawn steadybit config profile add +expect "Profile name:" { puts "REACHED_PROMPT" } +send "\003" +expect eof +catch wait result +exit [lindex $result 3] diff --git a/e2e/colour-on-tty.exp b/e2e/colour-on-tty.exp new file mode 100644 index 0000000..690d032 --- /dev/null +++ b/e2e/colour-on-tty.exp @@ -0,0 +1,13 @@ +# Colour is gated on stdout being a terminal, so the CLI has to be given one. Piping +# its output into a matcher would remove the very thing under test. +# +# The pattern is the escape character alone: expect matches globs by default, in which +# a "[" opens a character class rather than matching itself. +log_user 0 +set timeout 20 +spawn env STEADYBIT_TOKEN= steadybit experiment get -k ADM-1 +expect { + "\033" { catch { exp_close }; exit 0 } + eof { exit 1 } + timeout { exit 1 } +} diff --git a/e2e/run.sh b/e2e/run.sh new file mode 100755 index 0000000..ce9647a --- /dev/null +++ b/e2e/run.sh @@ -0,0 +1,101 @@ +#!/bin/sh +# SPDX-License-Identifier: MIT +# SPDX-FileCopyrightText: 2026 Steadybit GmbH + +# Smoke tests for the packaged CLI. Everything here needs a real process: an exit +# status, a terminal, or the spawn of a subcommand. Anything that can be asserted +# in-process belongs in the vitest suite instead, so these stay at the level of +# "did it exit correctly" rather than checking output in detail. + +set -u +failures=0 + +# Driving the prompts needs a pty allocator, which the shipped image has no reason to +# carry. Installed here rather than baked into a second image: a derived image has to +# name its base by tag, and a tag can silently resolve to something from a registry +# instead of the build under test. +if ! command -v expect >/dev/null 2>&1; then + apk add --no-cache expect >/dev/null 2>&1 || { + echo "cannot install expect, which the interactive checks need" + exit 1 + } +fi + +check() { + description=$1 + shift + if "$@"; then + echo " ok $description" + else + echo " FAIL $description" + failures=$((failures + 1)) + fi +} + +exits_with() { + expected=$1 + shift + "$@" >/dev/null 2>&1 + actual=$? + [ "$actual" -eq "$expected" ] || { + echo " expected exit $expected, got $actual" + return 1 + } +} + +echo "steadybit CLI container smoke tests" + +check "--version succeeds" exits_with 0 steadybit --version +check "--help succeeds" exits_with 0 steadybit --help +check "a subcommand is spawned and runs" exits_with 0 steadybit experiment --help +check "an unknown command fails" exits_with 1 steadybit definitely-not-a-command + +# The access token is resolved before anything else, so this is the guard on every +# platform-touching command. +check "a missing access token fails" exits_with 1 env STEADYBIT_TOKEN= steadybit experiment get -k ADM-1 +check "an unreachable platform fails" exits_with 1 \ + env STEADYBIT_TOKEN=t STEADYBIT_URL=http://127.0.0.1:1 steadybit experiment get -k ADM-1 + +# Colour is gated on stdout being a terminal. Both halves are checked here rather than +# depending on how the container was started: the pipe below is genuinely not a +# terminal, and expect genuinely provides one. +# Asserting only the absence of escapes would also hold if nothing were printed at all, +# so the output has to be there first. +check "output is clean when piped" sh -c ' + out=$(env STEADYBIT_TOKEN= steadybit experiment get -k ADM-1 2>&1) + # Something the CLI itself prints. Merely having output would also be satisfied by the + # shell reporting that there is no such command. + echo "$out" | grep -q "No API access token" || exit 1 + ! printf "%s" "$out" | grep -q "$(printf "\033")" +' +check "output is coloured on a terminal" expect /e2e/colour-on-tty.exp + +# The interactive flow, driven through a pty. Writes into the container's own home. +check "profile add stores what was typed" sh -c ' + expect /e2e/add-profile.exp >/dev/null 2>&1 || exit 1 + grep -q "\"name\": \"e2e\"" "$HOME/.steadybit/profiles.json" || exit 1 + steadybit config profile list | grep -q "e2e" +' + +check "ctrl-c during a prompt exits 130" sh -c ' + expect /e2e/cancel-profile.exp >/dev/null 2>&1 + [ $? -eq 130 ] +' + +# Same trap: no stack trace and no profile are both true of a CLI that never ran, so +# the prompt has to be shown to have been reached. +check "ctrl-c leaves no stack trace and no profile" sh -c ' + rm -rf "$HOME/.steadybit" + out=$(expect /e2e/cancel-profile.exp 2>&1) + echo "$out" | grep -q "REACHED_PROMPT" || exit 1 + echo "$out" | grep -q "ExitPromptError" && exit 1 + [ ! -f "$HOME/.steadybit/profiles.json" ] +' + +echo +if [ "$failures" -eq 0 ]; then + echo "all container smoke tests passed" +else + echo "$failures container smoke test(s) failed" +fi +exit "$failures" diff --git a/package-lock.json b/package-lock.json index e54f191..46bd80c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -25,6 +25,7 @@ }, "devDependencies": { "@eslint/js": "^10.0.1", + "@inquirer/testing": "^3.3.9", "@types/node": "^22.19.4", "@types/semver": "^7.3.9", "eslint": "^10.8.0", @@ -319,15 +320,6 @@ "node": ">= 12" } }, - "node_modules/@inquirer/core/node_modules/mute-stream": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/mute-stream/-/mute-stream-3.0.0.tgz", - "integrity": "sha512-dkEJPVvun4FryqBmZ5KhDo0K9iDXAwn08tMLDinNdRBNPcYEDiWYysLcc6k3mjTMlbP9KyylvRpd4wFtwrT9rw==", - "license": "ISC", - "engines": { - "node": "^20.17.0 || >=22.9.0" - } - }, "node_modules/@inquirer/core/node_modules/signal-exit": { "version": "4.1.0", "resolved": "https://registry.npmjs.org/signal-exit/-/signal-exit-4.1.0.tgz", @@ -415,6 +407,89 @@ } } }, + "node_modules/@inquirer/testing": { + "version": "3.3.9", + "resolved": "https://registry.npmjs.org/@inquirer/testing/-/testing-3.3.9.tgz", + "integrity": "sha512-083hjj4LYr0HrCGKgWsKaNCT7wFsLZVUN6oqvEXcmV0tXcQgnrQ8cqtTT18KCCSZyS0ANpQifrpbFpjrelaTmw==", + "dev": true, + "license": "MIT", + "dependencies": { + "@inquirer/type": "^4.0.7", + "@xterm/headless": "^6.0.0", + "mute-stream": "^3.0.0" + }, + "engines": { + "node": ">=23.5.0 || ^22.13.0 || ^20.17.0" + }, + "peerDependencies": { + "@inquirer/checkbox": ">=1.0.0", + "@inquirer/confirm": ">=1.0.0", + "@inquirer/editor": ">=1.0.0", + "@inquirer/expand": ">=1.0.0", + "@inquirer/external-editor": ">=1.0.0", + "@inquirer/input": ">=1.0.0", + "@inquirer/number": ">=1.0.0", + "@inquirer/password": ">=1.0.0", + "@inquirer/prompts": ">=1.0.0", + "@inquirer/rawlist": ">=1.0.0", + "@inquirer/search": ">=1.0.0", + "@inquirer/select": ">=1.0.0", + "@types/jest": ">=29.0.0", + "@types/node": ">=18", + "jest": ">=29.0.0", + "vitest": ">=1.0.0" + }, + "peerDependenciesMeta": { + "@inquirer/checkbox": { + "optional": true + }, + "@inquirer/confirm": { + "optional": true + }, + "@inquirer/editor": { + "optional": true + }, + "@inquirer/expand": { + "optional": true + }, + "@inquirer/external-editor": { + "optional": true + }, + "@inquirer/input": { + "optional": true + }, + "@inquirer/number": { + "optional": true + }, + "@inquirer/password": { + "optional": true + }, + "@inquirer/prompts": { + "optional": true + }, + "@inquirer/rawlist": { + "optional": true + }, + "@inquirer/search": { + "optional": true + }, + "@inquirer/select": { + "optional": true + }, + "@types/jest": { + "optional": true + }, + "@types/node": { + "optional": true + }, + "jest": { + "optional": true + }, + "vitest": { + "optional": true + } + } + }, "node_modules/@inquirer/type": { "version": "4.0.7", "resolved": "https://registry.npmjs.org/@inquirer/type/-/type-4.0.7.tgz", @@ -1067,6 +1142,16 @@ "url": "https://opencollective.com/vitest" } }, + "node_modules/@xterm/headless": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/@xterm/headless/-/headless-6.0.0.tgz", + "integrity": "sha512-5Yj1QINYCyzrZtf8OFIHi47iQtI+0qYFPHmouEfG8dHNxbZ9Tb9YGSuLcsEwj9Z+OL75GJqPyJbyoFer80a2Hw==", + "dev": true, + "license": "MIT", + "workspaces": [ + "addons/*" + ] + }, "node_modules/acorn": { "version": "8.18.0", "resolved": "https://registry.npmjs.org/acorn/-/acorn-8.18.0.tgz", @@ -2263,6 +2348,15 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/mute-stream": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/mute-stream/-/mute-stream-3.0.0.tgz", + "integrity": "sha512-dkEJPVvun4FryqBmZ5KhDo0K9iDXAwn08tMLDinNdRBNPcYEDiWYysLcc6k3mjTMlbP9KyylvRpd4wFtwrT9rw==", + "license": "ISC", + "engines": { + "node": "^20.17.0 || >=22.9.0" + } + }, "node_modules/nanoid": { "version": "3.3.16", "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.16.tgz", diff --git a/package.json b/package.json index 855a683..913ab34 100644 --- a/package.json +++ b/package.json @@ -54,6 +54,7 @@ }, "devDependencies": { "@eslint/js": "^10.0.1", + "@inquirer/testing": "^3.3.9", "@types/node": "^22.19.4", "@types/semver": "^7.3.9", "eslint": "^10.8.0", diff --git a/src/config/profile/add.test.ts b/src/config/profile/add.test.ts new file mode 100644 index 0000000..6cbec68 --- /dev/null +++ b/src/config/profile/add.test.ts @@ -0,0 +1,105 @@ +// SPDX-License-Identifier: MIT +// SPDX-FileCopyrightText: 2026 Steadybit GmbH + +import fs from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { wrapPrompt } from '@inquirer/testing/vitest'; +import { answerPrompt, waitForPrompt } from '../../mocks/prompts.ts'; +import { add } from './add.ts'; +import type { Profile } from './types.ts'; + +vi.mock('@inquirer/input', async importOriginal => { + const actual = await importOriginal(); + return { ...actual, default: wrapPrompt(actual.default) }; +}); +vi.mock('@inquirer/password', async importOriginal => { + const actual = await importOriginal(); + return { ...actual, default: wrapPrompt(actual.default) }; +}); + +// The flow writes a real profile store, so HOME points at a scratch directory. +const fakeHome = await fs.mkdtemp(path.join(os.tmpdir(), 'steadybit-add-test-')); +process.env.HOME = fakeHome; +if (!os.homedir().startsWith(fakeHome)) { + throw new Error(`refusing to run: home directory is ${os.homedir()}, not the scratch directory`); +} + +async function storedProfiles(): Promise { + return JSON.parse(await fs.readFile(path.join(fakeHome, '.steadybit', 'profiles.json'), 'utf8')); +} + +describe('config profile add', () => { + beforeEach(() => { + // Otherwise the flow wipes the test runner's output. + vi.spyOn(console, 'clear').mockImplementation(() => undefined); + vi.spyOn(console, 'log').mockImplementation(() => undefined); + }); + + it('should ask for a name, a base url and a token, and store them', async () => { + const done = add({} as never); + + await answerPrompt('Profile name:', 'from-the-prompt'); + await answerPrompt('Base URL of the Steadybit server:', 'https://platform.example.com'); + await answerPrompt('API access token:', 's3cr3t'); + await done; + + expect(await storedProfiles()).toContainEqual({ + name: 'from-the-prompt', + baseUrl: 'https://platform.example.com', + apiAccessToken: 's3cr3t', + }); + }); + + it('should fall back to the public platform when the base url is left empty', async () => { + const done = add({} as never); + + await answerPrompt('Profile name:', 'defaulted'); + await answerPrompt('Base URL of the Steadybit server:', ''); // accept the offered default + await answerPrompt('API access token:', 'tok'); + await done; + + expect((await storedProfiles()).find(p => p.name === 'defaulted')?.baseUrl).toEqual( + 'https://platform.steadybit.com' + ); + }); + + it('should refuse a blank name and ask again', async () => { + const done = add({} as never); + + await answerPrompt('Profile name:', ' '); + await waitForPrompt('You must provide a valid value'); + + await answerPrompt('Profile name:', 'eventually-valid', { replace: true }); + await answerPrompt('Base URL of the Steadybit server:', ''); + await answerPrompt('API access token:', 'tok'); + await done; + + expect((await storedProfiles()).map(p => p.name)).toContain('eventually-valid'); + }); + + it('should refuse a base url that is not http', async () => { + const done = add({} as never); + + await answerPrompt('Profile name:', 'bad-url'); + await answerPrompt('Base URL of the Steadybit server:', 'ftp://files.example.com'); + await waitForPrompt('Unsupported protocol ftp:'); + + await answerPrompt('Base URL of the Steadybit server:', 'https://platform.example.com', { replace: true }); + await answerPrompt('API access token:', 'tok'); + await done; + + expect((await storedProfiles()).map(p => p.name)).toContain('bad-url'); + }); + + it('should skip the questions entirely when name and token are given', async () => { + await add({ name: 'non-interactive', token: 'tok', baseUrl: 'https://given.example.com' }); + + expect(await storedProfiles()).toContainEqual({ + name: 'non-interactive', + baseUrl: 'https://given.example.com', + apiAccessToken: 'tok', + }); + }); +}); diff --git a/src/config/profile/select.test.ts b/src/config/profile/select.test.ts new file mode 100644 index 0000000..63e99a5 --- /dev/null +++ b/src/config/profile/select.test.ts @@ -0,0 +1,53 @@ +// SPDX-License-Identifier: MIT +// SPDX-FileCopyrightText: 2026 Steadybit GmbH + +import fs from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; +import { beforeAll, describe, expect, it, vi } from 'vitest'; +import { wrapPrompt } from '@inquirer/testing/vitest'; +import { answerPrompt, waitForPrompt } from '../../mocks/prompts.ts'; +import { addProfile } from './service.ts'; +import { select } from './select.ts'; + +vi.mock('@inquirer/select', async importOriginal => { + const actual = await importOriginal(); + return { ...actual, default: wrapPrompt(actual.default) }; +}); + +const fakeHome = await fs.mkdtemp(path.join(os.tmpdir(), 'steadybit-select-test-')); +process.env.HOME = fakeHome; +if (!os.homedir().startsWith(fakeHome)) { + throw new Error(`refusing to run: home directory is ${os.homedir()}, not the scratch directory`); +} + +const activeProfileFile = path.join(fakeHome, '.steadybit', 'activeProfile'); + +describe('config profile select', () => { + beforeAll(async () => { + await addProfile({ name: 'alpha', apiAccessToken: 'a' }); + await addProfile({ name: 'beta', apiAccessToken: 'b' }); + }); + + it('should offer every configured profile', async () => { + const done = select(); + + await waitForPrompt('Choose the new active profile:'); + await waitForPrompt('alpha'); + await waitForPrompt('beta'); + + await answerPrompt('Choose the new active profile:', ''); + await done; + }); + + it('should make the chosen profile the active one', async () => { + const done = select(); + + await waitForPrompt('Choose the new active profile:'); + // Down to the second entry, then accept. + await answerPrompt('Choose the new active profile:', '\x1b[B'); + await done; + + expect(await fs.readFile(activeProfileFile, 'utf8')).toEqual('beta'); + }); +}); diff --git a/src/config/profile/service.test.ts b/src/config/profile/service.test.ts index 52fff88..a671458 100644 --- a/src/config/profile/service.test.ts +++ b/src/config/profile/service.test.ts @@ -6,17 +6,17 @@ import os from 'node:os'; import path from 'node:path'; import { beforeAll, describe, expect, it, vi } from 'vitest'; -// service.ts resolves the config directory from the home directory at module load, so -// HOME is redirected to a scratch directory before importing it. The guard below refuses -// to run the suite if that ever stops working, since these tests write profile files. +import { addProfile, getActiveProfile, getProfiles, setActiveProfile } from './service.ts'; + +// These tests write profile files, so HOME is redirected to a scratch directory. The +// guard refuses to run the suite if that ever stops working. A plain import is enough +// because service.ts resolves the config directory per call rather than at module load. const fakeHome = await fs.mkdtemp(path.join(os.tmpdir(), 'steadybit-service-test-')); process.env.HOME = fakeHome; if (!os.homedir().startsWith(fakeHome)) { throw new Error(`refusing to run: home directory is ${os.homedir()}, not the scratch directory`); } -const { addProfile, getActiveProfile, getProfiles, setActiveProfile } = await import('./service.ts'); - const profilesFile = path.join(fakeHome, '.steadybit', 'profiles.json'); describe('profile service', () => { @@ -59,4 +59,43 @@ describe('profile service', () => { expect((await getProfiles()).map(p => p.name)).toContain('third'); }); + + // The config directory used to be computed at module load, which meant nothing could + // point the CLI at a different home once this module had been imported. + it('should follow a home directory that changes after import', async () => { + const otherHome = await fs.mkdtemp(path.join(os.tmpdir(), 'steadybit-other-home-')); + const previous = process.env.HOME; + process.env.HOME = otherHome; + + try { + await setActiveProfile('written-elsewhere'); + expect(await fs.readFile(path.join(otherHome, '.steadybit', 'activeProfile'), 'utf8')).toEqual( + 'written-elsewhere' + ); + } finally { + process.env.HOME = previous; + } + }); + + // The directory memo followed HOME while the read memo did not, so a changed home + // produced correct directories with the previous home's contents in them. + it('should read from a home directory that changes after import', async () => { + const otherHome = await fs.mkdtemp(path.join(os.tmpdir(), 'steadybit-other-home-')); + await fs.mkdir(path.join(otherHome, '.steadybit'), { recursive: true }); + await fs.writeFile( + path.join(otherHome, '.steadybit', 'profiles.json'), + JSON.stringify([{ name: 'only-over-here', apiAccessToken: 'z' }]) + ); + + const previous = process.env.HOME; + process.env.HOME = otherHome; + try { + expect((await getProfiles()).map(p => p.name)).toEqual(['only-over-here']); + } finally { + process.env.HOME = previous; + } + + // ...and the original home is still answered correctly afterwards. + expect((await getProfiles()).map(p => p.name)).toContain('first'); + }); }); diff --git a/src/config/profile/service.ts b/src/config/profile/service.ts index b3e8bab..346c578 100644 --- a/src/config/profile/service.ts +++ b/src/config/profile/service.ts @@ -8,31 +8,45 @@ import path from 'node:path'; import { abortExecution, errorMessage } from '../../errors.ts'; import type { Profile } from './types.ts'; -const configDir = path.join(homedir(), '.steadybit'); -const profilesFile = path.join(configDir, 'profiles.json'); -const activeProfileFile = path.join(configDir, 'activeProfile'); +// Resolved per call rather than at module load. HOME is what decides where the profile +// store lives, and a test — or anything else setting it after this module is imported — +// would otherwise be talking to the developer's real ~/.steadybit. +const configDir = () => path.join(homedir(), '.steadybit'); +const profilesFile = () => path.join(configDir(), 'profiles.json'); +const activeProfileFile = () => path.join(configDir(), 'activeProfile'); // The profile files are read on every API call, three times per call via -// getConfiguration(). Reading them once per process turns thousands of redundant -// syscalls into a handful during commands like `experiment dump`. Failures are not -// cached, so they stay retryable, and the writers below drop the cache. -function readOnce(read: () => Promise): (() => Promise) & { forget: () => void } { - let cached: Promise | undefined; - const cachingRead = () => { - cached ??= read().catch(e => { - cached = undefined; - throw e; - }); - return cached; +// getConfiguration(). Doing the work once turns thousands of redundant syscalls into a +// handful during commands like `experiment dump`. +// +// Remembered against the config directory rather than outright, because that directory +// follows HOME: a single cached value would answer for whichever home happened to be +// current first, and for the directory-creation entry that meant leaving a later +// directory unmade while every write into it failed. Failures are not remembered, so an +// unreadable or unwritable home stays retryable, and the writers below forget the entry +// for the home they wrote to. +function oncePerConfigDirectory(work: () => Promise): (() => Promise) & { forget: () => void } { + const done = new Map>(); + const runOnce = () => { + const directory = configDir(); + let result = done.get(directory); + if (!result) { + result = work().catch(e => { + done.delete(directory); + throw e; + }); + done.set(directory, result); + } + return result; }; - cachingRead.forget = () => { - cached = undefined; + runOnce.forget = () => { + done.delete(configDir()); }; - return cachingRead; + return runOnce; } -const ensureConfigDirectoryExists = readOnce(async () => { - await fs.mkdir(configDir, { recursive: true }); +const ensureConfigDirectoryExists = oncePerConfigDirectory(async () => { + await fs.mkdir(configDir(), { recursive: true }); }); export async function addProfile(profile: Profile): Promise { @@ -51,24 +65,24 @@ export async function removeProfile(profileName: string): Promise { await writeProfiles(updatedProfiles); } -const readProfiles = readOnce(async (): Promise => { +const readProfiles = oncePerConfigDirectory(async (): Promise => { await ensureConfigDirectoryExists(); let fileContent: string; try { - fileContent = await fs.readFile(profilesFile, { encoding: 'utf8' }); + fileContent = await fs.readFile(profilesFile(), { encoding: 'utf8' }); } catch (e) { if ((e as any)?.code === 'ENOENT') { return []; } - throw abortExecution("Failed to read file '%s': %s", profilesFile, errorMessage(e)); + throw abortExecution("Failed to read file '%s': %s", profilesFile(), errorMessage(e)); } try { return JSON.parse(fileContent); } catch (e) { - throw abortExecution("Failed to parse file '%s' as JSON: %s", profilesFile, errorMessage(e)); + throw abortExecution("Failed to parse file '%s' as JSON: %s", profilesFile(), errorMessage(e)); } }); @@ -80,22 +94,22 @@ async function writeProfiles(profiles: Profile[]): Promise { await ensureConfigDirectoryExists(); try { - await fs.writeFile(profilesFile, JSON.stringify(profiles, undefined, 2)); + await fs.writeFile(profilesFile(), JSON.stringify(profiles, undefined, 2)); } catch (e) { - throw abortExecution("Failed to write to file '%s': %s", profilesFile, errorMessage(e)); + throw abortExecution("Failed to write to file '%s': %s", profilesFile(), errorMessage(e)); } readProfiles.forget(); } -const readActiveProfileName = readOnce(async (): Promise => { +const readActiveProfileName = oncePerConfigDirectory(async (): Promise => { await ensureConfigDirectoryExists(); try { // Users opening and saving the file might end up adding a trailing new line character. - return (await fs.readFile(activeProfileFile, { encoding: 'utf8' })).trim(); + return (await fs.readFile(activeProfileFile(), { encoding: 'utf8' })).trim(); } catch (e) { if ((e as any)?.code !== 'ENOENT') { - throw abortExecution("Failed to read file '%s': %s", profilesFile, errorMessage(e)); + throw abortExecution("Failed to read file '%s': %s", activeProfileFile(), errorMessage(e)); } return undefined; } @@ -112,9 +126,9 @@ export async function setActiveProfile(profileName: string): Promise { await ensureConfigDirectoryExists(); try { - await fs.writeFile(activeProfileFile, profileName); + await fs.writeFile(activeProfileFile(), profileName); } catch (e) { - throw abortExecution("Failed to write to file '%s': %s", activeProfileFile, errorMessage(e)); + throw abortExecution("Failed to write to file '%s': %s", activeProfileFile(), errorMessage(e)); } readActiveProfileName.forget(); } diff --git a/src/experiment/exec.interactive.test.ts b/src/experiment/exec.interactive.test.ts new file mode 100644 index 0000000..1e37ade --- /dev/null +++ b/src/experiment/exec.interactive.test.ts @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: MIT +// SPDX-FileCopyrightText: 2026 Steadybit GmbH + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { wrapPrompt } from '@inquirer/testing/vitest'; +import { answerPrompt } from '../mocks/prompts.ts'; +import { givenAnotherExperimentIsRunning } from '../mocks/handlers.ts'; +import { executeExperiments } from './exec.ts'; + +vi.mock('@inquirer/confirm', async importOriginal => { + const actual = await importOriginal(); + return { ...actual, default: wrapPrompt(actual.default) }; +}); + +// These go through the prompts rather than past them. exec.test.ts covers the same +// commands with no terminal attached, which is what CI does and what the +// defaultWhenNonInteractive values are about; both branches matter. +describe('experiment run, with a terminal', () => { + let logged: string[]; + + beforeEach(() => { + Object.defineProperty(process.stdout, 'isTTY', { value: true, configurable: true }); + logged = []; + vi.spyOn(console, 'log').mockImplementation((...args) => { + logged.push(args.join(' ')); + }); + }); + + afterEach(() => { + Object.defineProperty(process.stdout, 'isTTY', { value: undefined, configurable: true }); + vi.restoreAllMocks(); + }); + + it('should not run anything when the confirmation is declined', async () => { + // Throwing rather than returning, so that the flow stops here the way a real exit + // would. A no-op stub lets execution carry on and run the experiment anyway. + const exit = vi.spyOn(process, 'exit').mockImplementation(code => { + throw new Error(`exited with ${code}`); + }); + + const done = executeExperiments({ key: 'TST-1', recursive: false }); + await answerPrompt('Are you sure you want to run the experiment?', 'n'); + + await expect(done).rejects.toThrow('exited with 0'); + expect(exit).toHaveBeenCalledWith(0); + expect(logged.join('\n')).not.toContain('Executing experiment'); + }); + + // This recovery was unreachable until the response body it inspects stopped being + // consumed before it got there, so it has never been exercised end to end. + it('should offer to run in parallel when another experiment is already running', async () => { + givenAnotherExperimentIsRunning(); + + const done = executeExperiments({ key: 'TST-1', recursive: false }); + await answerPrompt('Are you sure you want to run the experiment?', 'y'); + await answerPrompt('There is already an experiment running', 'y'); + await done; + + expect(logged.join('\n')).toContain('Executing experiment: TST-1'); + }); + + it('should give up when running in parallel is declined', async () => { + givenAnotherExperimentIsRunning(); + + const done = executeExperiments({ key: 'TST-1', recursive: false }); + await answerPrompt('Are you sure you want to run the experiment?', 'y'); + await answerPrompt('There is already an experiment running', 'n'); + + await expect(done).rejects.toThrow('Failed to run experiment (TST-1)'); + expect(logged.join('\n')).not.toContain('Executing experiment'); + }); +}); diff --git a/src/mocks/handlers.ts b/src/mocks/handlers.ts index fd2be22..61dc33b 100644 --- a/src/mocks/handlers.ts +++ b/src/mocks/handlers.ts @@ -12,6 +12,13 @@ let experimentStore: Record = {}; let validationFailuresRemaining = 0; let executionsPerExperiment: Record = {}; let unfetchableExecutions = new Set(); +let anotherExperimentRunning = false; + +// Lets a test reach the "an experiment is already running, run it in parallel?" recovery, +// which the platform only offers when the caller has not already asked for parallel. +export const givenAnotherExperimentIsRunning = () => { + anotherExperimentRunning = true; +}; function executionsFor(key: string): { id: number }[] { return (executionsPerExperiment[key] ?? []).map(id => ({ id })); @@ -31,6 +38,7 @@ export const resetExperiments = () => { validationFailuresRemaining = 0; executionsPerExperiment = {}; unfetchableExecutions = new Set(); + anotherExperimentRunning = false; }; export const setValidationFailures = (count: number) => { @@ -186,6 +194,18 @@ const executeExperimentHandler = http.post('http://example.com/api/experiments/: ); } + if (anotherExperimentRunning && requestUrl.searchParams.get('allowParallel') !== 'true') { + return HttpResponse.json( + { + type: 'https://steadybit.com/problems/another-experiment-running-exception', + title: 'Another experiment is currently running.', + status: 409, + instance: `/api/experiments/${params.key}/execute`, + }, + { status: 409 } + ); + } + const run = runSequence++; if (experiment) { return HttpResponse.json( diff --git a/src/mocks/prompts.ts b/src/mocks/prompts.ts new file mode 100644 index 0000000..529cb74 --- /dev/null +++ b/src/mocks/prompts.ts @@ -0,0 +1,36 @@ +// SPDX-License-Identifier: MIT +// SPDX-FileCopyrightText: 2026 Steadybit GmbH + +import { screen } from '@inquirer/testing/vitest'; + +// A prompt is not always on screen the moment the call under test starts: confirm() +// reaches its prompt through a dynamic import, and a rejected answer re-renders the +// same question. Polling for the text covers both without the caller having to know +// which case it is in, and reports the screen it did see when the wait runs out. +export async function waitForPrompt(text: string, timeoutMillis = 4000): Promise { + const deadline = Date.now() + timeoutMillis; + while (Date.now() < deadline) { + if (screen.getScreen().includes(text)) { + return; + } + await new Promise(resolve => setTimeout(resolve, 5)); + } + throw new Error(`Prompt "${text}" never appeared. Last screen was:\n${screen.getScreen()}`); +} + +// Enter is a carriage return; readline does not submit on a newline. `replace` clears +// what is already in the field first, which a prompt keeps after rejecting an answer — +// without it a second attempt is appended to the first rather than replacing it. +export async function answerPrompt(prompt: string, answer: string, { replace = false } = {}): Promise { + await waitForPrompt(prompt); + if (replace) { + screen.input.write('\x7f'.repeat(64)); // backspace past anything already typed + } + screen.input.write(`${answer}\r`); +} + +export function pressCtrlC(): void { + // Written as an escape rather than the literal byte, which is invisible in an editor + // and reads as an empty string. + screen.input.write('\x03'); +} diff --git a/src/prompt/cancellation.test.ts b/src/prompt/cancellation.test.ts new file mode 100644 index 0000000..51e4877 --- /dev/null +++ b/src/prompt/cancellation.test.ts @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: MIT +// SPDX-FileCopyrightText: 2026 Steadybit GmbH + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { wrapPrompt } from '@inquirer/testing/vitest'; +import { pressCtrlC, waitForPrompt } from '../mocks/prompts.ts'; +import { confirm } from './confirm.ts'; +import { cancelable } from './cancellation.ts'; + +vi.mock('@inquirer/confirm', async importOriginal => { + const actual = await importOriginal(); + return { ...actual, default: wrapPrompt(actual.default) }; +}); + +describe('cancelable', () => { + beforeEach(() => { + Object.defineProperty(process.stdout, 'isTTY', { value: true, configurable: true }); + }); + + afterEach(() => { + Object.defineProperty(process.stdout, 'isTTY', { value: undefined, configurable: true }); + vi.restoreAllMocks(); + }); + + // inquirer used to re-raise SIGINT; the @inquirer prompts reject instead, which would + // otherwise surface as an unhandled rejection with a stack trace. Previously this was + // only covered by throwing a hand-made ExitPromptError, never by a real cancellation. + it('should exit quietly with the conventional SIGINT status when the user cancels', async () => { + const exit = vi.spyOn(process, 'exit').mockImplementation((() => undefined) as never); + + // process.exit is stubbed, so cancelable() falls through to its rethrow instead of + // ending the process. In production it never gets that far. + void confirm('Run it?').catch(() => undefined); + await waitForPrompt('Run it?'); + pressCtrlC(); + + await vi.waitFor(() => expect(exit).toHaveBeenCalledWith(130)); + }); + + it('should let every other failure through untouched', async () => { + const boom = new Error('something else'); + + await expect(cancelable(Promise.reject(boom))).rejects.toBe(boom); + }); + + it('should pass a normal answer straight through', async () => { + await expect(cancelable(Promise.resolve('answered'))).resolves.toEqual('answered'); + }); +}); diff --git a/src/prompt/confirm.test.ts b/src/prompt/confirm.test.ts new file mode 100644 index 0000000..4395406 --- /dev/null +++ b/src/prompt/confirm.test.ts @@ -0,0 +1,53 @@ +// SPDX-License-Identifier: MIT +// SPDX-FileCopyrightText: 2026 Steadybit GmbH + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { wrapPrompt } from '@inquirer/testing/vitest'; +import { answerPrompt, waitForPrompt } from '../mocks/prompts.ts'; +import { confirm } from './confirm.ts'; + +// confirm() reaches the prompt through a dynamic import, so the mock has to survive +// `await import(...)` rather than only a static one. +vi.mock('@inquirer/confirm', async importOriginal => { + const actual = await importOriginal(); + return { ...actual, default: wrapPrompt(actual.default) }; +}); + +describe('confirm', () => { + describe('without a terminal', () => { + it('should answer with the non-interactive default instead of prompting', async () => { + await expect(confirm('Run it?', { defaultWhenNonInteractive: false })).resolves.toBe(false); + await expect(confirm('Run it?', { defaultWhenNonInteractive: true })).resolves.toBe(true); + }); + }); + + describe('with a terminal', () => { + beforeEach(() => { + // The guard in confirm() decides whether to prompt at all. Faking it is the only + // way in-process; whether the guard reads the real terminal correctly is left to + // the container tests, which get a genuine tty from `docker run -t`. + Object.defineProperty(process.stdout, 'isTTY', { value: true, configurable: true }); + }); + + afterEach(() => { + Object.defineProperty(process.stdout, 'isTTY', { value: undefined, configurable: true }); + }); + + it('should take the answer the user gives', async () => { + const answer = confirm('Run it?', { defaultYes: false }); + + await answerPrompt('Run it?', 'y'); + + await expect(answer).resolves.toBe(true); + }); + + it('should offer the configured default', async () => { + const answer = confirm('Run it?', { defaultYes: false }); + + await waitForPrompt('(y/N)'); // the configured default is the one offered + await answerPrompt('Run it?', ''); + + await expect(answer).resolves.toBe(false); + }); + }); +});