S223 : fix deps#29
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
There was a problem hiding this comment.
Pull request overview
Updates dependencies and modernizes the test setup to address Dependabot alerts and move off Mocha/Should.
Changes:
- Upgrade runtime dependencies (notably
@google-cloud/datastore) and refresh the lockfile. - Replace Mocha/Should tests with Vitest and add Vitest configuration + scripts.
- Update Node engine requirement and bump GitHub Actions workflow action versions.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
vitest.config.cjs |
Adds Vitest config (Node environment, .test.mjs include glob). |
test/datastore-backup.test.mjs |
Replaces Mocha test with Vitest for datastoreRestoreCommand. |
test/datastore-backup.js |
Removes legacy Mocha/Should test. |
test/cmd.test.mjs |
Replaces Mocha tests with Vitest for CLI helper functions. |
test/cmd.js |
Removes legacy Mocha/Should tests. |
package.json |
Switches test scripts to Vitest, updates deps, adds Node engine + overrides. |
package-lock.json |
Lockfile refresh for upgraded deps and Vitest/Vite toolchain. |
lib/cmd.js |
Ensures colors side-effects are loaded for msg.red usage. |
AGENTS.md |
Updates documentation to reflect Vitest test framework. |
.github/workflows/test.yml |
Updates action versions used in CI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "type": "Apache-2.0", | ||
| "url": "https://opensource.org/licenses/apache2.0.php" | ||
| "engines": { | ||
| "node": ">=20" |
There was a problem hiding this comment.
engines.node is broader than what the updated test/tooling stack supports. The lockfile pulls in vite@8 (via Vitest) which requires Node ^20.19.0 || >=22.12.0, so values like Node 20.0–20.18, 21.x, and 23.x would satisfy >=20 but fail installs/runs. Tighten the engine range to match the actual minimum/supported Node versions (or adjust dependency versions accordingly).
| "node": ">=20" | |
| "node": "^20.19.0 || >=22.12.0" |
| ).toBe('project_backups_daily'); | ||
| }); | ||
|
|
||
| it('generates bucket id from project-id, if program does not specificy prefix', () => { |
There was a problem hiding this comment.
Typo in test description: "specificy" -> "specify".
| it('generates bucket id from project-id, if program does not specificy prefix', () => { | |
| it('generates bucket id from project-id, if program does not specify prefix', () => { |
| it('produces expected command', () => { | ||
| const cmd = datastoreRestoreCommand( | ||
| ['EntityA', 'EntityB'], | ||
| 'gae-project', | ||
| 'gae-project_backup', | ||
| '2020-01-01T00:00:00Z', | ||
| {}, | ||
| ); |
There was a problem hiding this comment.
The new test only covers the default options = {} case. In lib/datastore-backup.js, the options.account branch currently concatenates --account without a leading space (and the status command path looks malformed as well), which would generate invalid gcloud commands when an account is provided. Add a test case that passes an account option and asserts the expected spacing/format so this doesn’t regress (and fix the underlying command builder to satisfy the test).
davidfq
left a comment
davidfq
left a comment
There was a problem hiding this comment.
OK;
one improvement: consolidate everything to ES6 modules:
vitest.config.mjsinstead ofvitest.config.cjs/lib.cmd.jsand/lib/datastore-backup.jsexported as named modules so we don't need thecreateRequirefunction
Fixes
Change implications