test: add unit test suite reaching 80% coverage on all metrics (cascades on #30)#31
Merged
vitorfdl merged 18 commits intofeat/cli-refactoring-depsfrom Apr 27, 2026
Merged
Conversation
Expands the test suite from 35 to 358 passing tests, lifting line
coverage from ~45% to 80.16% (above the SPEC threshold of 80%).
Coverage per layer:
- lib/: 89.8%
- prompt/: 95.5%
- commands/: 74.3%
- profile/backup/resources/: 84.5%
Patterns standardized across the suite:
- Proxy-based Account/Resources mock via makeAccount (shared helper)
- makeEnvironmentConfig factory for config stubs
- resetInjectedPrompts between tests to avoid prompts.inject() leakage
- installFetchMock + makeFetchResponse/ArrayBuffer/Stream helpers
- Dynamic `await import("./foo.js")` so vi.mock() factories hoist cleanly
- Fake timers (vi.useFakeTimers) for async.queue resources with
setTimeout-based pacing
Notes on known gaps:
- `src/commands/profile/backup/restore.ts` (9% covered) exercises axios
streams + unzipper in ways that are not easily mocked without a real
zip fixture; left as-is.
- `src/commands/start-config.ts` (21%) has deeply interactive branches
that would need heavy prompt scripting to cover fully.
- One latent `async.queue.drain()` hang when the queue never receives
items is documented in fixtures (all test fixtures force at least one
item to avoid triggering it).
CI impact: `npm run linter` → 0/0 on 170 files; `npm test` → 358/358.
Expands existing test files across the suite to lift branch coverage from 73.9% to 80.01%, closing the last remaining SPEC threshold. Coverage per metric (final): - Lines 86.63% - Statements 86.49% - Functions 82.50% - Branches 80.01% All four thresholds ≥ 80%, matching the SPEC acceptance criteria. Files expanded (targeting highest-ROI branch gaps): - backup/resources/*: add granularItem selection flows (10 files) - profile/export/services: collect-ids, widgets-export, dashboards-export, devices-export, analysis-export, actions-export - profile/export/export.ts: per-entity isolation tests that exercise the "collect dependencies first" branches that single-entity runs trigger (devices alone, dashboards alone, access alone, etc.) - profile/backup/create.ts: non-ok POST, custom API base, null-backup poll continue - profile/backup/lib.ts: 2FA-cancel and pin-cancel paths - commands: login (URL helper branches + deploy URL derivation), deploy (analysis.info reject, upload failure, default paths), duplicate-analysis (pick cancellation, fetch non-ok), change-network (both prompts empty, partial override), copy-tab (prompt for dashboard id, tab pick cancel, no arrangement) - lib: token (empty folder branch), config-file (create-on-missing, unparseable file, writeConfigFileEnv, writeToConfigFile, setDefault, resolveCLIPath), notify-update (fetch helper + getLatestVersion) - devices: data-post (prompt path, fallback to Device lookup, silent-fail branch), analysis-console (scriptObj unresolved, onmessage, onopen, onerror) Restore.ts (9% coverage) and start-config.ts (21%) intentionally left as-is: - restore.ts: streams + unzipper + disk I/O + ~13 cascaded SDK calls across every backup resource would require a real zip fixture and is fragile to mock. - start-config.ts: deeply interactive prompt chains with tmpdir filesystem walks. Extensive prompt scripting would be brittle. CI: switch `code-quality.yml` from `npm test` to `npm run test:coverage` so the 80% threshold is actually enforced, and bump `actions/checkout` and `actions/setup-node` to v6.
Adds --all to deploy every analysis in tagoconfig.json without any prompt, and -t/--token to supply a profile token at invocation time (bypassing the lock file, which doesn't exist in CI runners). Also rejects the legacy "deploy all" positional with a helpful pointer to --all. The "all" positional has never worked as the help text suggested — it opened an interactive prompt — so dropping it is a bug fix, not a breaking change. Help examples updated: remove the misleading "deploy all" line and add pipeline-friendly usages covering --all, --all + stage-env, and the full CI combination (--all -e prod -t $TAGOIO_TOKEN --silent). Surgical scope: no changes to init, login, or any shared infrastructure. Only deploy.ts and analysis/index.ts are touched.
Documents the deploy flow for CI runners using --all + -e + -t + --silent, including the install step for @tago-io/cli and @tago-io/builder (deploy shells out to analysis-builder, so the builder needs to be on PATH). Notes that -t accepts either a profile token or an external-analysis token with the proper Access Management permissions, and that a pre-configured tagoconfig.json in the repository is required.
Adds coverage for the four new behaviors:
- legacy positional "all" is rejected with a pointer to --all
- --all deploys every analysis with no prompt
- -t/--token overrides the lock-file token for the run
- --all + -t end-to-end works with no lock file (CI flow)
Also updates the existing "env missing" test to reflect the new
error message ("No profile token found" instead of "Environment not
found") and introduces a defaultOptions() helper to DRY up the test
setup now that IDeployOptions has 5 required fields.
…p test assertions
…gister `tagoio run <name>` was broken on every Node run: the command called `node -r <swc-register-path>` via `resolveCLIPath`, but that package was removed during Phase 1 when SWC was ripped out. Every invocation failed with `Cannot find module '.../node_modules/@swc-node/register/index'`. Switches the Node path in `_buildCMD` to `node --experimental-strip-types --watch`. Node 24+ strips TypeScript type annotations natively, so no external loader is required. The flag is stable on Node 24 and does not emit a runtime warning (unlike `--experimental-transform-types`, which does). Limitation: strip-types does not handle `enum` or `namespace` — not used in customer analyses today. Also: - Drops the now-unused `resolveCLIPath` import from run-analysis.ts. - Updates the three `_buildCMD` tests that asserted the old SWC path. - README: removes the `.swcrc` / `.swcrc.example` note from the Analysis Runner section (the debugger no longer needs source-map config — Node's strip-types preserves line numbers). - README (CI/CD): documents the Access Management rule required when using an external-analysis token in `-t, --token` (Access Analysis + Edit Analysis + Upload Analysis Script), so least-privilege pipelines don't hit Authorization Denied at deploy time. - `analysis/index.ts`: spells out `--env` instead of `-e` in the help examples for consistency with the option name shown above the list.
`tagoio run <name>` was broken for legacy analysis projects that use
`"module": "CommonJS"` + `"moduleResolution": "Node"` in their tsconfig
and import subpaths without a `.js` extension (e.g.
`import { Data } from "@tago-io/sdk/lib/types"`). Commit 60c5792 had
swapped the runner to `node --experimental-transform-types --watch`, but
Node's built-in TS execution routes those files through the ESM loader,
which rejects extensionless specifiers with `ERR_MODULE_NOT_FOUND`.
Every legacy analysis failed at import time.
Switches `_buildCMD` to invoke tsx's CLI via the cached binary shipped
inside the CLI's own node_modules:
`node <cli-path>/node_modules/tsx/dist/cli.mjs watch <script>`. tsx
pairs esbuild (TS transpile) with oxc-resolver and CJS-aware module
resolution, so the same classic imports resolve cleanly. Watch mode is
provided by `tsx watch` instead of `node --watch`, and `--inspect` /
`--clear` continue to work.
Why tsx over reviving `@swc-node/register`:
- Install footprint: ~12 MB per platform (esbuild single native binary)
vs ~25-36 MB for @swc/core + oxc-resolver
- One native binary to ship, not two
- Watch + REPL ship built-in
- Same resolver internally (oxc-resolver), so no correctness difference
- 10x the weekly npm downloads and stronger maintenance signal
Also:
- Adds `tsx@^4.21.0` to dependencies.
- Reintroduces the `resolveCLIPath` import removed in 60c5792.
- Updates the three `_buildCMD` tests that asserted the Node native
flags to assert the tsx CLI path and `watch` subcommand instead.
The `--tsnd` and `--deno` flags are unchanged.
Device restore was only recreating the device record itself. Two pieces of state that users expect to survive a backup/restore cycle were silently dropped: - Configuration parameters (`device.params`) — these carry per-device runtime settings and are required by most integrations. - Tokens tied to a physical device by `serie_number` — without these, the device cannot communicate after restore. Adds three helpers in `devices.ts`: - `stripDeviceFields` centralizes the list of fields the TagoIO API rejects on create/edit (ids, timestamps, profile, and the two restored-separately fields `params` and `tokens`). Both `processCreateTask` and `processEditTask` now delegate to it, removing the previously duplicated destructuring. - `restoreDeviceParams` replays the backup's params via the dedicated `paramSet` endpoint. - `restoreDeviceTokens` recreates only tokens with a `serie_number` (the identifying attribute for physical devices); ephemeral tokens without one are intentionally skipped. On the edit path it fetches the current token list first and skips serials already present, so re-runs are idempotent. Token values cannot be preserved because the backup stores them masked — callers must update integrations that relied on the old value. Tests cover: param restoration on create/edit, serial-token recreation, skipping tokens without a serial, idempotency when a serial already exists on the target device, and graceful handling of a failed `tokenCreate` (logged, restore continues).
feat(cli): CI/CD flags, fix legacy TS runner, restore device params and tokens (cascades on #31)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Cascading PR on top of #30. Adds the full unit-test suite and wires CI to enforce coverage, fulfilling the SPEC requirement of ≥ 80% across all four metrics.
Merges only after #30 lands. Contains no production-code changes — only tests, test utilities, and one CI workflow bump.
Coverage result
npx vitest run --coverageexits 0 on this branch.What this PR contains
Commit 1 —
test: add 71 unit test files to reach 80% line coverageAdds the initial test suite from scratch: 71 new files, 323 tests. Lifts line coverage from baseline to ~80%.
Commit 2 —
test: raise branch coverage above 80% and wire CI to test:coverageExpands 26 existing test files to close branch gaps, bringing all four metrics above 80%. Also updates
.github/workflows/code-quality.ymlto runnpm run test:coverage(notnpm test), so the threshold is enforced on every push.Patterns used across the suite
Account/Resourcesmock viamakeAccount(added in refactor: modernize toolchain, migrate to ESM, and remove axios/lodash #30).makeEnvironmentConfigfactory for config stubs.resetInjectedPrompts()between tests to preventprompts.inject()leakage across cases.installFetchMock+ helpers (makeFetchResponse,makeFetchArrayBufferResponse,makeFetchStreamResponse) for the fetch-migrated code paths.await import("./foo.js")sovi.mock()factories hoist cleanly when the module depends on mocked globals or SDK constructors. Note: there's a potential follow-up cleanup PR to convert mock-function-access patterns into top-level mock constants — left out of this PR to keep scope tight.vi.useFakeTimers) forasync.queueresources withsetTimeout-based pacing.Known gaps (documented, not blocking the 80% thresholds)
src/commands/profile/backup/restore.ts— 9% branches. Testing would need: a real ZIP fixture streamed throughfetch, coordinatingpipeline()+unzipper.Extract().promise()on disk, and ~13 cascaded SDK mocks across every backup resource type. Fragile and high LOC for low marginal value. Deferred.src/commands/start-config.ts— 17% branches. Deeply interactive (12+ chainedprompts()calls, realreaddirSyncfilesystem walks, multi-step login loopback). Covering well would require long, brittleprompts.inject([...])sequences plus tmpdir setup. Deferred.async.queue.drain()hang when the queue never receives items (source behavior, not introduced by this PR). All test fixtures force at least one item into the queue to avoid triggering it.Test plan
test:coverage+ build).npx vitest run --coverage→ exits 0, all 4 thresholds ≥ 80%.