chore(cli): cover placeholder filtering in local autoversioning - #17326
chore(cli): cover placeholder filtering in local autoversioning#17326devin-ai-integration[bot] wants to merge 2 commits into
Conversation
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
There was a problem hiding this comment.
AI Review Summary
Test-only PR adding regression coverage for placeholder-version filtering in LocalTaskHandler.usableVersion. The three cases are well-targeted and the failure-injection verification described is convincing. Only minor cleanliness nits: unused helper flexibility and a dead branch in the mock helper.
- 🔵 2 suggestion(s)
| function metadataReturns(version: string | undefined) { | ||
| mockLoggingExeca.mockImplementation((_logger: unknown, cmd: string, args: string[]) => { | ||
| if (cmd === "git" && args[0] === "show" && args[1] === "HEAD:.fern/metadata.json") { | ||
| return Promise.resolve({ | ||
| stdout: version == null ? "" : JSON.stringify({ sdkVersion: version }), | ||
| exitCode: version == null ? 128 : 0 | ||
| }); | ||
| } | ||
| return Promise.resolve({ stdout: "", exitCode: 0 }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🔵 suggestion
The version == null branch (empty stdout + exitCode 128) is never exercised by any of the three tests, and it's also a slightly dubious simulation — loggingExeca typically rejects on non-zero exit unless reject: false is passed, so a future test relying on this would test a shape that never occurs. Consider dropping the undefined case until something needs it, narrowing the param to string.
| mockLoggingExeca.mockResolvedValue({ stdout: "", exitCode: 0 }); | ||
| }); | ||
|
|
||
| async function createTaskHandler(overrides: Record<string, unknown> = {}) { |
There was a problem hiding this comment.
🔵 suggestion
overrides is never used by any caller. Either drop the parameter or hoist/reuse the equivalent helper that the earlier describe blocks in this file already use, rather than defining a fourth near-identical factory.
Docs Generation Benchmark ResultsComparing PR branch against median of 5 nightly run(s) on
Docs generation runs |
SDK Generation Benchmark ResultsComparing PR branch against median of 5 nightly run(s) on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Claude Code Review
Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.
Tip: disable this comment in your organization's Code Review settings.
Description
Follow-up to #17320, which stopped
fern generate --localfrom treating0.0.0-fern-placeholderas a real previous version. That PR shipped theLocalTaskHandlerguard without a permanent test (it was only verified with a throwaway probe); this adds the regression coverage.Changes Made
describeblock inLocalTaskHandler.fallback.test.tsasserting theusableVersionguard: a placeholder from the diff falls through to.fern/metadata.json(1.5.0→1.5.1), an already-mutated0.0.0-fern-placeholder.0also falls through (2.0.0→2.1.0), and a placeholder in every source lands on the initial0.0.1instead of0.0.0-fern-placeholder.0.Testing
pnpm vitest run --dir .../local-workspace-runner/src/__test__→ 17 files, 183 passed. Temporarily neutering theisPlaceholderVersion(...)check inLocalTaskHandler.usableVersionmakes exactly the 3 new tests fail, confirming they pin the guard rather than passing incidentally.pnpm checkclean.Link to Devin session: https://app.devin.ai/sessions/0dfb28eb8c1447df9207a0b676a81931
Requested by: @fern-support