test: add integrated test for cli #5

syi0808 · 2024-10-23T02:34:52Z

Summary by CodeRabbit

New Features
- Added landing page with features showcase, quickstart guide, comparison table, and interactive terminal preview.
- CLI now supports --help/-h and --version/-v flags.
Bug Fixes
- Improved error resilience when querying registries.
- Fixed registry detection logic for npm-only scenarios.
Tests
- Expanded test coverage with CLI, Git, package management, and registry tests.
Chores
- Updated dev dependencies (vitest, coverage tools).
- Added test fixtures and refined coverage configuration.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

github-actions · 2024-10-23T02:35:17Z

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	96.72%	1124 / 1162
🔵	Statements	96.72%	1124 / 1162
🔵	Functions	100%	110 / 110
🔵	Branches	94.68%	303 / 320

File Coverage

File	Stmts	Branches	Functions	Lines	Uncovered Lines
Changed Files
src/options.ts	100%	100%	100%	100%

Generated in workflow #150 for commit 2b5ab7a by the Vitest Coverage Report Action

Copilot

Copilot reviewed 6 out of 10 changed files in this pull request and generated 1 comment.

Files not reviewed (4)

package.json: Language not supported
pnpm-lock.yaml: Language not supported
tests/fixtures/build-script/package.json: Language not supported
tests/fixtures/test-script/package.json: Language not supported

Comments suppressed due to low confidence (2)

tests/utils/cli.ts:120

Converting runPubmCli from an async to a synchronous function may introduce timing issues if callers rely on awaiting subprocess completion; ensure that the subprocess exit is awaited or that tests are updated accordingly.

export function runPubmCli(

src/tasks/runner.ts:38

The revised logic for determining npmOnly is stricter than before and may not account for mixed registry configurations; please confirm that this behavior is intentional.

npmOnly: options.registries.every((registry) => registry === 'npm'),

src/tasks/required-missing-information.ts

coderabbitai · 2026-01-03T12:19:56Z

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

This PR introduces a comprehensive landing page (docs/landing.html) with interactive features, adds extensive unit and integration test coverage (15+ test files), updates testing infrastructure (vitest/coverage v3.1.2, new UI), refactors CLI and options handling to canonicalize paths and resolve options consistently, improves error handling in registry operations, and adds test fixtures for various package configurations.

Changes

Cohort / File(s)	Summary
Documentation & Landing Page `docs/landing.html`	New static landing page with hero section, features grid, comparison table, quickstart code blocks with copy-to-clipboard, fixed navigation, scroll-reveal animations, and terminal typing effect. Embedded CSS for dark theme and responsive design.
Project Configuration `.gitignore`, `package.json`, `vitest.config.mts`	Added "coverage" to gitignore; bumped vitest and `@vitest/coverage-v8` from v2.1.1 to v3.1.2; added `@vitest/ui` ^3.1.2; changed test/coverage scripts to remove `--run` flag; refined coverage includes to `src/*/.ts` and excludes `src/types/`, `src/cli.ts`, `src/tasks/`.
CLI & Options Refactoring `src/cli.ts`, `src/options.ts`, `src/tasks/runner.ts`	Resolves CLI options to absolute path when contents provided; moved directory change to occur after option resolution; reversed options/defaults merge precedence so user options take priority; narrowed npmOnly detection to exactly 'npm' registries only; removed redundant directory change from runner.
Error Handling & Resilience `src/tasks/required-missing-information.ts`	Wrapped distTags computation in try/catch to gracefully handle registry errors without throwing.
Test Infrastructure & Utilities `tests/utils/cli.ts`	Refactored runPubmCli from async command-based to synchronous path-based runner; added UP and ENTER key constants; returns direct controller object with exitCode, stdout, stderr, and waitForClose.
Integration Tests `tests/cli.test.ts`	New CLI integration test suite verifying --help, --version, -h, -v flags with stdout assertions.
Unit Tests - Core `tests/unit/index.test.ts`, `tests/unit/options.test.ts`, `tests/unit/listr.test.ts`	Tests for pubm entry point, options resolution with merge precedence, and Listr utility.
Unit Tests - Utilities `tests/unit/db.test.ts`, `tests/unit/error.test.ts`, `tests/unit/engine-version.test.ts`, `tests/unit/git.test.ts`, `tests/unit/notify-new-version.test.ts`, `tests/unit/package.test.ts`, `tests/unit/package-manager.test.ts`, `tests/unit/package-name.test.ts`, `tests/unit/rollback.test.ts`, `tests/unit/utils-cli.test.ts`	Comprehensive unit test coverage for database operations, error handling, engine validation, git operations, version notifications, package utilities, package manager detection, package name validation, rollback mechanics, and CLI utilities.
Unit Tests - Registries `tests/unit/registry/index.test.ts`, `tests/unit/registry/npm.test.ts`, `tests/unit/registry/jsr.test.ts`, `tests/unit/registry/custom-registry.test.ts`, `tests/unit/registry/registry.test.ts`	Unit tests for registry abstraction, npm registry operations, jsr registry and client, custom registry with URL override, and registry factory.
Test Fixtures `tests/fixtures/both-json/`, `tests/fixtures/build-script/`, `tests/fixtures/jsr-json/`, `tests/fixtures/package-json/`, `tests/fixtures/test-script/*`	New fixture manifests for testing multi-manifest packages (npm+jsr), build scripts, jsr-only packages, standard npm packages, and test scripts.
Removed Test `tests/test.test.ts`	Deleted trivial placeholder test.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

The PR spans heterogeneous changes: a full landing page with embedded interactivity, 15+ unit test files with mocked dependencies, test fixture additions, CLI/options refactoring affecting control flow, dependency version bumps, and coverage configuration. Each test file and core logic change requires separate reasoning; however, test files follow consistent patterns, reducing overall review density.

Poem

🐰 A landing page emerges, gleaming bright,
Tests bloom in clusters through the night,
Options now resolve with paths so true,
And registries handle errors anew—
Pubm hops forward, shiny and new! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'test: add integrated test for cli' accurately describes the main changes in the PR. The changeset includes a new CLI test file (tests/cli.test.ts) and numerous supporting test infrastructure additions, making this a test-focused PR.

✨ Finishing touches

📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 19

🧹 Nitpick comments (9)

docs/landing.html (2)

13-778: Consider extracting CSS to an external file.

The embedded CSS is well-structured, but at 765+ lines, it affects maintainability and prevents browser caching. For production use, consider moving this to an external stylesheet.

1049-1107: Consider extracting JavaScript to an external file.

Similar to the CSS, the inline JavaScript could be moved to an external file for better maintainability and browser caching. This is particularly beneficial if the landing page is frequently updated.
src/tasks/required-missing-information.ts (1)
75-81: Consider adding warning logging for distTags retrieval failures.

In a previous review, you mentioned you would "make a warning" when distTags retrieval fails. However, the current implementation silently catches errors without logging. This could make debugging registry issues difficult.
🔎 Suggested implementation with warning
 try {
   distTags = [
     ...new Set(
       (await Promise.all([npm.distTags(), jsr.distTags()])).flat(),
     ),
   ].filter((tag) => tag !== defaultOptions.tag);
-} catch {}
+} catch (error) {
+  console.warn('Failed to retrieve dist-tags from registry:', error);
+}
Based on past review comments where you indicated you would add warning logging for this error case.
tests/unit/listr.test.ts (1)
1-49: Consider adding test coverage for externalSignalHandler.

The implementation in src/utils/listr.ts (line 11) also sets the externalSignalHandler property. Consider adding a test to verify this property is correctly assigned.
🔎 Suggested test addition
+	test('sets externalSignalHandler', () => {
+		const listr = createListr([]);
+
+		// @ts-ignore - accessing patched property
+		expect(listr.externalSignalHandler).toBeDefined();
+	});
tests/unit/registry/npm.test.ts (1)
1-17: Import sorting issue flagged by CI.

The pipeline indicates imports should be sorted. Consider reordering to satisfy the linter.
Proposed fix
-import { describe, expect, test, vi, beforeEach, afterEach } from 'vitest';
-import { NpmRegistry, npmRegistry } from '../../../src/registry/npm';
-
-vi.mock('tinyexec', () => ({
-	exec: vi.fn(),
-}));
-
-vi.mock('../../../src/utils/package', () => ({
-	getPackageJson: vi.fn().mockResolvedValue({
-		name: 'test-package',
-		version: '1.0.0',
-	}),
-}));
-
-import { exec } from 'tinyexec';
+import { exec } from 'tinyexec';
+import { beforeEach, afterEach, describe, expect, test, vi } from 'vitest';
+
+import { NpmRegistry, npmRegistry } from '../../../src/registry/npm';
+
+vi.mock('tinyexec', () => ({
+	exec: vi.fn(),
+}));
+
+vi.mock('../../../src/utils/package', () => ({
+	getPackageJson: vi.fn().mockResolvedValue({
+		name: 'test-package',
+		version: '1.0.0',
+	}),
+}));
Note: The exact ordering depends on your project's lint rules. The vi.mock calls must remain hoisted, but the static imports can be reordered.
src/cli.ts (1)
1-12: Import sorting issue flagged by CI.

The pipeline indicates imports should be sorted. The path import on line 12 should be grouped with other node imports if a sorting convention is enforced.
Proposed fix
+import path from 'node:path';
 import cac from 'cac';
 import type { OptionConfig } from 'cac/deno/Option.js';
 import semver from 'semver';
 import { isCI } from 'std-env';
 import { consoleError } from './error.js';
 import { Git } from './git.js';
 import { pubm } from './index.js';
 import { requiredMissingInformationTasks } from './tasks/required-missing-information.js';
 import type { Options } from './types/options.js';
 import { notifyNewVersion } from './utils/notify-new-version.js';
 import { version } from './utils/package.js';
-import path from 'node:path';
tests/unit/index.test.ts (1)
1-21: Import sorting issue flagged by CI.

The pipeline indicates imports should be sorted.
Proposed fix
-import { describe, expect, test, vi, beforeEach } from 'vitest';
-
-vi.mock('../../src/options', () => ({
+import { beforeEach, describe, expect, test, vi } from 'vitest';
+
+import { pubm } from '../../src/index';
+import { resolveOptions } from '../../src/options';
+import { run } from '../../src/tasks/runner';
+
+vi.mock('../../src/options', () => ({
 	resolveOptions: vi.fn((options) => ({
 		...options,
 		testScript: options.testScript ?? 'test',
 		buildScript: options.buildScript ?? 'build',
 		branch: options.branch ?? 'main',
 		tag: options.tag ?? 'latest',
 		registries: options.registries ?? ['npm', 'jsr'],
 	})),
 }));
 
 vi.mock('../../src/tasks/runner', () => ({
 	run: vi.fn().mockResolvedValue(undefined),
 }));
-
-import { pubm } from '../../src/index';
-import { run } from '../../src/tasks/runner';
-import { resolveOptions } from '../../src/options';
Note: The exact ordering depends on your project's lint configuration.
tests/unit/registry/jsr.test.ts (1)
1-24: Import sorting issue flagged by CI.

The pipeline indicates imports should be sorted.
Proposed fix
-import { describe, expect, test, vi, beforeEach, afterEach } from 'vitest';
-import { JsrRegisry, JsrClient, jsrRegistry } from '../../../src/registry/jsr';
-
-vi.mock('tinyexec', () => ({
-	exec: vi.fn(),
-}));
+import { exec } from 'tinyexec';
+import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest';
+
+import { JsrClient, JsrRegisry, jsrRegistry } from '../../../src/registry/jsr';
 
+vi.mock('tinyexec', () => ({
+	exec: vi.fn(),
+}));
 vi.mock('../../../src/utils/package', () => ({
 ...
 }));
-
-import { exec } from 'tinyexec';
tests/utils/cli.ts (1)
1-5: Import sorting issue flagged by CI.

The pipeline indicates imports should be sorted. The node:path import should be grouped with other node imports.
Proposed fix
+import path from 'node:path';
 import type { Readable, Writable } from 'node:stream';
 import { stripVTControlCharacters } from 'node:util';
 import { isCI } from 'std-env';
 import { type Options, exec } from 'tinyexec';
-import path from 'node:path';

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 435d406 and 2b5ab7a.

⛔ Files ignored due to path filters (1)

pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

📒 Files selected for processing (35)

.gitignore
docs/landing.html
package.json
src/cli.ts
src/options.ts
src/tasks/required-missing-information.ts
src/tasks/runner.ts
tests/cli.test.ts
tests/fixtures/both-json/jsr.json
tests/fixtures/both-json/package.json
tests/fixtures/build-script/package.json
tests/fixtures/jsr-json/jsr.json
tests/fixtures/package-json/package.json
tests/fixtures/test-script/package.json
tests/test.test.ts
tests/unit/db.test.ts
tests/unit/engine-version.test.ts
tests/unit/error.test.ts
tests/unit/git.test.ts
tests/unit/index.test.ts
tests/unit/listr.test.ts
tests/unit/notify-new-version.test.ts
tests/unit/options.test.ts
tests/unit/package-manager.test.ts
tests/unit/package-name.test.ts
tests/unit/package.test.ts
tests/unit/registry/custom-registry.test.ts
tests/unit/registry/index.test.ts
tests/unit/registry/jsr.test.ts
tests/unit/registry/npm.test.ts
tests/unit/registry/registry.test.ts
tests/unit/rollback.test.ts
tests/unit/utils-cli.test.ts
tests/utils/cli.ts
vitest.config.mts

💤 Files with no reviewable changes (1)

tests/test.test.ts

🧰 Additional context used

🧬 Code graph analysis (19)

tests/unit/engine-version.test.ts (2)

src/utils/package.ts (1)

getPackageJson (49-90)

src/utils/engine-version.ts (1)

validateEngineVersion (5-16)

src/tasks/required-missing-information.ts (3)

src/registry/jsr.ts (2)

distTags (53-55)

jsr (35-41)

src/registry/npm.ts (2)

distTags (106-119)

npm (14-20)

src/options.ts (1)

defaultOptions (3-9)

tests/unit/utils-cli.test.ts (1)

src/utils/cli.ts (2)

warningBadge (3-3)

link (5-7)

tests/unit/db.test.ts (1)

src/utils/db.ts (1)

Db (20-58)

tests/unit/listr.test.ts (1)

src/utils/listr.ts (1)

createListr (4-17)

tests/unit/package-name.test.ts (1)

src/utils/package-name.ts (4)

isScopedPackage (3-5)

getScope (7-9)

getScopeAndName (11-17)

isValidPackageName (24-60)

tests/unit/package.test.ts (1)

src/utils/package.ts (7)

jsrJsonToPackageJson (178-204)

packageJsonToJsrJson (130-176)

findOutFile (24-47)

patchCachedJsrJson (17-22)

getPackageJson (49-90)

getJsrJson (92-128)

replaceVersion (223-256)

tests/unit/notify-new-version.test.ts (3)

src/utils/package.ts (3)

findOutFile (24-47)

getPackageJson (49-90)

getJsrJson (92-128)

src/utils/package-name.ts (1)

getScopeAndName (11-17)

src/utils/notify-new-version.ts (1)

notifyNewVersion (6-61)

tests/unit/index.test.ts (3)

src/index.ts (1)

pubm (15-19)

src/options.ts (1)

resolveOptions (11-15)

src/tasks/runner.ts (1)

run (34-246)

tests/unit/git.test.ts (1)

src/git.ts (7)

git (10-16)

Git (9-343)

branch (153-161)

status (107-115)

tags (36-48)

commit (286-296)

commits (117-141)

tests/utils/cli.ts (1)

src/index.ts (1)

Options (21-21)

tests/unit/registry/index.test.ts (1)

src/registry/index.ts (1)

getRegistry (12-22)

tests/unit/registry/custom-registry.test.ts (1)

src/registry/custom-registry.ts (2)

CustomRegistry (5-16)

customRegistry (18-22)

tests/unit/package-manager.test.ts (2)

src/utils/package.ts (1)

findOutFile (24-47)

src/utils/package-manager.ts (1)

getPackageManager (11-19)

tests/unit/error.test.ts (1)

src/error.ts (2)

AbstractError (3-12)

consoleError (42-54)

tests/unit/rollback.test.ts (1)

src/utils/rollback.ts (2)

addRollback (6-11)

rollback (15-27)

tests/unit/registry/npm.test.ts (2)

src/registry/npm.ts (2)

NpmRegistry (11-183)

npmRegistry (185-189)

src/utils/package.ts (1)

getPackageJson (49-90)

src/cli.ts (2)

src/utils/notify-new-version.ts (1)

notifyNewVersion (6-61)

src/index.ts (1)

pubm (15-19)

tests/unit/registry/jsr.test.ts (2)

src/registry/jsr.ts (3)

JsrRegisry (25-123)

JsrClient (125-300)

jsrRegistry (302-306)

src/utils/package.ts (1)

getJsrJson (92-128)

🪛 GitHub Actions: CI

tests/unit/engine-version.test.ts

Organize imports: Import statements could be sorted.

tests/unit/utils-cli.test.ts

[error] 1-1: Import statements could be sorted: current order differs from recommended.

tests/unit/db.test.ts

Organize imports: Import statements could be sorted.

tests/cli.test.ts

[error] 5-5: CLI help command shows usage timed out after 5000ms.

[error] 13-13: CLI version command shows version timed out after 5000ms.

[error] 19-19: CLI -h alias shows help timed out after 5000ms.

[error] 26-26: CLI -v alias shows version timed out after 5000ms.

tests/fixtures/both-json/package.json

[error] 1-1: Formatter would have printed the following content: - name: "@test/both-package" - version: "3.0.0"

tests/unit/index.test.ts

Organize imports: Import statements could be sorted.

tests/fixtures/package-json/package.json

[error] 1-1: Formatter would have printed the following content: - name: "@test/package" - version: "1.0.0" - description: "Test package for unit tests" - exports: { ": { import: "./dist/index.js", require: "./dist/index.cjs" }, "./utils": { import: "./dist/utils.js" } } - files: ["dist", "!dist/test", "src"] - engines: { node: ">=18" }

tests/fixtures/test-script/package.json

[error] 1-1: Formatter would have printed the following content: - name: "test-script-fixtures" - version: "0.0.1" - scripts: { test: "test" }

tests/fixtures/build-script/package.json

[error] 1-1: Formatter would have printed the following content: - name: "build-script-fixtures" - version: "0.0.1" - scripts: { build: "build" }

tests/utils/cli.ts

[error] 1-1: Import statements could be sorted: current order differs from recommended.

tests/unit/registry/index.test.ts

Organize imports: Import statements could be sorted.

tests/unit/registry/custom-registry.test.ts

[error] 43-43: lint/suspicious/noExplicitAny - Unexpected any. Specify a different type.

[error] 60-60: lint/suspicious/noExplicitAny - Unexpected any. Specify a different type.

tests/unit/error.test.ts

Organize imports: Import statements could be sorted.

tests/unit/rollback.test.ts

[error] 1-1: Import statements could be sorted: current order differs from recommended.

package.json

[error] 1-1: Formatter would have printed the following content: - name: "@test/both-package" - version: "3.0.0"

[error] 1-1: Formatter would have printed the following content: - name: "build-script-fixtures" - version: "0.0.1" - scripts: { build: "build" }

[error] 1-1: Formatter would have printed the following content: - name: "test-script-fixtures" - version: "0.0.1" - scripts: { test: "test" }

[error] 1-1: Formatter would have printed the following content: - name: "@test/package" - version: "1.0.0" - description: "Test package for unit tests" - exports: { ": { import: "./dist/index.js", require: "./dist/index.cjs" }, "./utils": { import: "./dist/utils.js" } } - files: ["dist", "!dist/test", "src"] - engines: { node: ">=18" }

tests/fixtures/both-json/jsr.json

[error] 1-1: Formatter would have printed the following content: - name: "@test/both-jsr" - version: "3.0.0" - exports: "./src/index.ts"

tests/unit/registry/npm.test.ts

[error] 1-1: Import statements could be sorted: current order differs from recommended.

src/cli.ts

[error] 1-1: Import statements could be sorted: current order differs from recommended. Suggested reordering.

[error] 1-1: Import statements could be sorted: current order differs from recommended.

tests/fixtures/jsr-json/jsr.json

[error] 1-1: Formatter would have printed the following content: - name: "@test/jsr-package" - version: "2.0.0" - exports: { ".": "./src/index.ts", "./utils": "./src/utils.ts" } - publish: { include: ["src"], exclude: ["src/test"] }

tests/unit/registry/jsr.test.ts

Organize imports: Import statements could be sorted.

🪛 GitHub Check: Coverage: node-20, ubuntu-latest

tests/cli.test.ts