🐛 Update import to match module type and add test coverage to make sure all modules load. by markstos · Pull Request #693 · TryGhost/gctools

markstos · 2026-02-27T02:47:43Z

Adds test coverage to make sure all modules can load.

coderabbitai · 2026-02-27T02:48:01Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 216024a and 2dd88c3.

📒 Files selected for processing (1)

test/cli-smoke.test.js

🚧 Files skipped from review as they are similar to previous changes (1)

test/cli-smoke.test.js

Walkthrough

This pull request adds a new integration test file test/cli-smoke.test.js that validates the CLI can execute the --help command without errors. The test creates a helper function to execute the CLI via Node's execFile, runs it with the --help argument, and asserts that the process exits with code 0, produces no stderr output, and returns stdout containing usage information. No production code changes are included.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 4

❌ Failed checks (1 warning, 3 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title mentions two key aspects—updating an import for module type and adding test coverage—but the changes only show test additions without the import fix.	Clarify whether the import fix in tasks/zip-split.js is included in this PR or if only the test was added.
Linked Issues check	❓ Inconclusive	The PR adds a smoke test validating CLI --help loads without errors, addressing issue `#692`'s requirement for regression prevention, but the critical import fix for tasks/zip-split.js appears incomplete.	Confirm that tasks/zip-split.js has been updated with the default import and destructuring pattern as required by issue `#692` to fully resolve the SyntaxError.
Out of Scope Changes check	❓ Inconclusive	The smoke test is appropriate for the stated objectives, but the import fix requirement from issue `#692` is missing from the changeset.	Verify that the import change to tasks/zip-split.js is present; if missing, this is a gap rather than out-of-scope work.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description is related to the changeset but provides minimal detail about what the test does or how it validates module loading.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

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: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tasks/zip-split.js`:
- Around line 7-8: The import uses a default import pattern that will fail at
runtime; replace the default-style import of the package with a named import for
the exported symbol by changing the import of superbytes so it uses the named
export (refer to the import statement that currently does `import pkg from
'superbytes'` and the referenced identifier `superbytes`) so the module is
imported as the named export instead of a default.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 47a6cc8 and f7bd725.

📒 Files selected for processing (1)

tasks/zip-split.js

tasks/zip-split.js

cursor · 2026-02-27T14:19:32Z

You have run out of free Bugbot PR reviews for this billing cycle. This will reset on March 3.

To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

coderabbitai

🧹 Nitpick comments (1)

test/cli-smoke.test.js (1)

10-20: Consider handling timeout/signal termination cases.

When execFile fails due to timeout (or signal), error.code may be undefined while error.signal or error.killed are set. This could cause exitCode to be undefined rather than a meaningful value, potentially masking timeout failures in test assertions.

For a --help smoke test this is unlikely to occur, but for robustness:

♻️ Suggested improvement

 function runCLI(args) {
     return new Promise((resolve) => {
         execFile('node', [cliPath, ...args], {timeout: 10000}, (error, stdout, stderr) => {
             resolve({
-                exitCode: error ? error.code : 0,
+                exitCode: error ? (error.code ?? 1) : 0,
+                killed: error?.killed ?? false,
                 stdout,
                 stderr
             });
         });
     });
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/cli-smoke.test.js` around lines 10 - 20, The runCLI helper currently
sets exitCode to error.code which can be undefined for timeouts/signals; modify
runCLI (the function that calls execFile with cliPath) to detect error.signal or
error.killed when error is truthy and map those cases to a deterministic
exitCode (for example 124 for timeout or 1 for killed) and include that mapped
code in the resolved object so tests never get an undefined exitCode.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@test/cli-smoke.test.js`:
- Around line 10-20: The runCLI helper currently sets exitCode to error.code
which can be undefined for timeouts/signals; modify runCLI (the function that
calls execFile with cliPath) to detect error.signal or error.killed when error
is truthy and map those cases to a deterministic exitCode (for example 124 for
timeout or 1 for killed) and include that mapped code in the resolved object so
tests never get an undefined exitCode.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f7bd725 and 216024a.

📒 Files selected for processing (1)

test/cli-smoke.test.js

PaulAdamDavis · 2026-03-02T20:17:58Z

Hey @markstos, I've just allowed tests to run on this, and it's getting the same failure as I do locally 🤔

test at test/cli-smoke.test.js:23:5
✖ --help loads all commands without errors (324.466167ms)
  AssertionError [ERR_ASSERTION]: CLI exited with code 1.
  stderr: file:///Users/paul/Sites/gctools/tasks/zip-split.js:7
  import pkg from 'superbytes';
         ^^^
  SyntaxError: The requested module 'superbytes' does not provide an export named 'default'
      at ModuleJob._instantiate (node:internal/modules/esm/module_job:226:21)
      at async ModuleJob.run (node:internal/modules/esm/module_job:335:5)
      at async onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:665:26)
      at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:117:5)

  Node.js v22.21.1

The superbytes docs themselves suggest importing like how this originally was, which suggests the issue is maybe local to you?

markstos · 2026-03-02T20:22:55Z

The superbytes docs themselves suggest importing like how this originally was, which suggests the issue is maybe local to you?

Does ./bin/cli.js --help work for you without the proposed change? It was not covered by the test suite.

PaulAdamDavis · 2026-03-02T20:27:10Z

@markstos I get the same error whether I run the tests or the file directly.

╭─ ~/Sites/gctools  pr/693:issue-692-superbytes *3 ····································· 1 ✘  20:24:02
╰─ yarn
yarn install v1.22.22
[1/5] 🔍  Validating package.json...
[2/5] 🔍  Resolving packages...
success Already up-to-date.
✨  Done in 0.10s.

╭─ ~/Sites/gctools  pr/693:issue-692-superbytes *3 ····································· ✔  20:24:10
╰─ ./bin/cli.js --help
file:///Users/paul/Sites/gctools/tasks/zip-split.js:7
import pkg from 'superbytes';
       ^^^
SyntaxError: The requested module 'superbytes' does not provide an export named 'default'
    at ModuleJob._instantiate (node:internal/modules/esm/module_job:226:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:335:5)
    at async onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:665:26)
    at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:117:5)

Node.js v22.21.1

markstos · 2026-03-02T20:35:22Z

I tested on a second computer. --help fails on the main branch but works on this branch after the proposed change:

mark@hex:~/g/gctools|main⚡?
➤ ./bin/cli.js --help
file:///home/mark/git/gctools/tasks/zip-split.js:7
import {superbytes} from 'superbytes';
        ^^^^^^^^^^
SyntaxError: Named export 'superbytes' not found. The requested module 'superbytes' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'superbytes';
const {superbytes} = pkg;

    at ModuleJob._instantiate (node:internal/modules/esm/module_job:180:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:263:5)
    at async onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:578:26)
    at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:116:5)

Node.js v22.14.0
mark@hex:~/g/gctools|main⚡?
➤ git checkout issue-692-superbytes 
Switched to branch 'issue-692-superbytes'
mark@hex:~/g/gctools|issue-692-superbytes⚡?
➤ ./bin/cli.js --help
Command line utilities for working with Ghost content
...

markstos · 2026-03-02T20:37:13Z

I tested once more with the same Node version as you, 22.21.1. Same result: fails on main, succeeds on this branch.

Before the test suite failed to catch a failure that would be obvious to a user that the "--help" command doesn't work, because no test checked simply that all modules could be loaded, which --help happens to do.

coderabbitai bot reviewed Feb 27, 2026

View reviewed changes

tasks/zip-split.js Outdated Show resolved Hide resolved

markstos changed the title ~~🐛 Update import to match module type.~~ 🐛 Update import to match module type and add test coverage to make sure all modules load. Feb 27, 2026

coderabbitai bot reviewed Feb 27, 2026

View reviewed changes

PaulAdamDavis mentioned this pull request Mar 2, 2026

✨ Make find-replace safer, support dry-run mode and more useful --verbose #694

Merged

test: Add "smoke test" that all sub-modules can be loaded.

2dd88c3

Before the test suite failed to catch a failure that would be obvious to a user that the "--help" command doesn't work, because no test checked simply that all modules could be loaded, which --help happens to do.

markstos force-pushed the issue-692-superbytes branch from 216024a to 2dd88c3 Compare March 2, 2026 20:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

🐛 Update import to match module type and add test coverage to make sure all modules load.#693

🐛 Update import to match module type and add test coverage to make sure all modules load.#693
markstos wants to merge 1 commit intoTryGhost:mainfrom
markstos:issue-692-superbytes

markstos commented Feb 27, 2026 •

edited

Loading

Uh oh!

coderabbitai bot commented Feb 27, 2026 •

edited

Loading

❌ Failed checks (1 warning, 3 inconclusive)

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

cursor bot commented Feb 27, 2026

Uh oh!

coderabbitai bot left a comment

Uh oh!

PaulAdamDavis commented Mar 2, 2026

Uh oh!

markstos commented Mar 2, 2026

Uh oh!

PaulAdamDavis commented Mar 2, 2026

Uh oh!

markstos commented Mar 2, 2026

Uh oh!

markstos commented Mar 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

markstos commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Estimated code review effort

❌ Failed checks (1 warning, 3 inconclusive)

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cursor bot commented Feb 27, 2026

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

PaulAdamDavis commented Mar 2, 2026

Uh oh!

markstos commented Mar 2, 2026

Uh oh!

PaulAdamDavis commented Mar 2, 2026

Uh oh!

markstos commented Mar 2, 2026

Uh oh!

markstos commented Mar 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

markstos commented Feb 27, 2026 •

edited

Loading

coderabbitai bot commented Feb 27, 2026 •

edited

Loading