feat: additional tests for higher code coverage#170
Conversation
WalkthroughThis pull request transitions end-to-end testing from simulator-based approaches to a macOS-native workflow via self-hosted runners, introduces server daemonization with JSON-RPC shutdown support, adds comprehensive Go and TypeScript unit tests, updates CI/CD workflows, and implements coverage collection infrastructure. Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/server.ts (1)
289-294:⚠️ Potential issue | 🟡 MinorCorrect the JSON-RPC method name in graceful shutdown approach.
The server supports graceful shutdown via JSON-RPC (method
server.shutdown), and the binary is instrumented withGOCOVERDIRfor coverage collection. Sending a graceful shutdown command beforeSIGTERMwill ensure coverage profiles are flushed properly.💡 Suggested approach
-function stopTestServer(): void { - if (serverProcess && !serverProcess.killed) { - serverProcess.kill(); - serverProcess = null; - } +async function stopTestServer(): Promise<void> { + if (serverProcess && !serverProcess.killed) { + try { + // Attempt graceful shutdown via JSON-RPC to flush coverage data + await axios.post(`${TEST_SERVER_URL}/rpc`, JSON.stringify({ + jsonrpc: '2.0', + method: 'server.shutdown', + id: 999 + }), { headers: { 'Content-Type': 'application/json' }, timeout: 3000 }); + } catch { + // Fall back to SIGTERM + serverProcess.kill(); + } + serverProcess = null; + } }
🤖 Fix all issues with AI agents
In @.github/workflows/build.yml:
- Around line 203-225: The publish job currently does not depend on the e2e_test
job, allowing releases to proceed even if E2E fails; update the publish job's
needs list (reference the publish job and the e2e_test job names) to include
e2e_test so publish depends on it, or if omission is intentional add an inline
comment next to the publish job explaining why e2e_test is excluded (e.g., flaky
self-hosted runner) to document the decision.
In `@devices/android_test.go`:
- Around line 290-319: The test TestAndroidDevice_PressButton_KeyMap is
self-referential and validates a locally defined keyMap instead of the real
mapping used by PressButton in android.go; update the test to exercise
production code by either (A) using the device's actual key map variable (make
it package-accessible or export it) and assert entries match validKeys, or (B)
preferibly instantiate an AndroidDevice, inject a mock/stub ADB client, call
d.PressButton(key) for each validKeys and assert the mock received the expected
KEYCODE string (verify calls to the ADB send/exec method), targeting the
PressButton method and the production key map (the key map variable used by
PressButton) to ensure real mapping is tested.
In `@server/dispatch.go`:
- Line 36: The server.shutdown endpoint is currently unauthenticated (invoked
via handleServerShutdown) and can be abused when CORS/external listen is
enabled; update handleServerShutdown to enforce authorization or restrict
callers: either validate an Authorization header (e.g., check a bearer
token/verify JWT or shared secret from request headers against server config)
before performing shutdown, or verify the remote address is a loopback/localhost
address and reject non-local requests; also apply the same protection to other
privileged handlers (e.g., device.reboot) so privileged operations require the
same auth or localhost-only check.
In `@utils/file_test.go`:
- Around line 47-48: The test currently ignores errors from os.Stat when
populating srcInfo and dstInfo which can be nil and cause a panic; update the
calls in utils/file_test.go so you check the returned error from os.Stat for
both srcPath and dstPath and call t.Fatal (with the error) if either returns
non-nil, ensuring srcInfo and dstInfo are valid before they are used on line 50.
🧹 Nitpick comments (20)
test/server.ts (2)
22-23: Fixed coverage directory may cause conflicts with parallel runs.Using a fixed
"coverage"directory name instead of a UUID-based one means multiple concurrent test runs would write to the same directory, potentially corrupting coverage data. If parallel execution is not a concern in your CI setup, this is acceptable.
36-39:afterhook should beasyncifstopTestServerbecomes async.Currently
stopTestServeris synchronous, but if you adopt the graceful shutdown approach above, theaftercallback needs to beasynctoawaitthe shutdown.utils/github_test.go (1)
10-44: Test name is misleading — this tests JSON parsing, notGetLatestReleaseDownloadURL.The test name suggests it exercises the
GetLatestReleaseDownloadURLfunction, but it only tests JSON serialization of theGitHubReleasestruct via a mock server. The comment on line 26 acknowledges this ("override the URL by testing the JSON parsing directly"). Consider renaming toTestGitHubRelease_ParsesJSONor similar to avoid confusion.cli/server.go (1)
50-70: Consider setting the flag default directly instead of a manual empty-string check.Both
serverStartCmdandserverKillCmdmanually check for an emptylistenflag and fall back todefaultServerAddress. You could simplify by passingdefaultServerAddressas the flag's default value, which also lets Cobra auto-document it in--help.♻️ Suggested simplification
- serverStartCmd.Flags().String("listen", "", "Address to listen on (e.g., 'localhost:12000' or '0.0.0.0:13000')") + serverStartCmd.Flags().String("listen", defaultServerAddress, "Address to listen on (e.g., 'localhost:12000' or '0.0.0.0:13000')")- serverKillCmd.Flags().String("listen", "", fmt.Sprintf("Address of server to kill (default: %s)", defaultServerAddress)) + serverKillCmd.Flags().String("listen", defaultServerAddress, "Address of server to kill")Then the
if addr == ""/if listenAddr == ""guards in bothRunEfunctions become unnecessary.utils/zipfile_test.go (1)
98-105: Unchecked error returns fromWriteandClosein test setup.Lines 103–105 (and similarly 126–128) discard the return values from
fw.Write,w.Close, andf.Close. If any of these fail, the test proceeds with a malformed zip and may pass or fail for the wrong reason. Consider checking errors (as you already do increateTestZip) for consistency and test reliability.♻️ Suggested fix (same pattern for the dot-dot test)
- fw.Write([]byte("evil")) - w.Close() - f.Close() + if _, err := fw.Write([]byte("evil")); err != nil { + t.Fatal(err) + } + if err := w.Close(); err != nil { + t.Fatal(err) + } + if err := f.Close(); err != nil { + t.Fatal(err) + }Makefile (1)
30-43: Coverage target body length (checkmake warning).The static analysis tool flags the
coveragetarget as exceeding 5 lines. The target performs a single logical pipeline (merge → format → report), and splitting it would hurt readability. This is a pragmatic exception to the rule.daemon/daemon.go (2)
55-71: Address normalization could double-prependhttp://if input already includes scheme.Line 71 unconditionally prepends
http://. If a caller ever passes"http://localhost:12000", the result would be"http://http://localhost:12000". Currently the CLI controls the input, but a defensive check is cheap:🛡️ Suggested guard
- // prepend http:// scheme - addr = "http://" + addr + // prepend http:// scheme if missing + if !strings.HasPrefix(addr, "http://") && !strings.HasPrefix(addr, "https://") { + addr = "http://" + addr + }
93-99: Error-string matching for "connection refused" is fragile.
strings.Contains(err.Error(), "connection refused")works on most platforms but is locale- and implementation-dependent. This is a common Go pattern and acceptable here, but be aware it may not match on all OS/locale combinations.docs/TESTING.md (1)
85-96: Good skip-behavior documentation, but note the CI implication.The "always succeeds" design is great for local dev, but means that if devices become misconfigured on the self-hosted runner, CI will silently pass with everything skipped. Consider adding a CI-specific mechanism (e.g., an env var or flag) to fail if expected device suites produce zero tests, ensuring the runner's device health is validated.
.github/workflows/build.yml (1)
207-225: No Node.js setup step ine2e_test.Unlike the
server_testandpublishjobs,e2e_testdoes not include anactions/setup-nodestep. It relies on the self-hosted runner having Node.js pre-installed. This is fine for a controlled self-hosted runner, but if the runner is ever reprovisioned or a different version is needed, tests could break silently. Consider addingactions/setup-nodefor reproducibility, or add a version-check step.devices/avd_test.go (1)
68-71: Uset.Setenvinstead of manualos.Setenv+deferfor safer env manipulation.The manual save-restore pattern on lines 69–71 (repeated in every AVD test) is fragile—if the test panics between
Setenvand the deferred restore, or if tests are ever parallelized, the HOME variable leaks.t.Setenv(available since Go 1.17) handles cleanup automatically and panics if misused witht.Parallel().♻️ Suggested refactor (apply to all similar occurrences)
- origHome := os.Getenv("HOME") - os.Setenv("HOME", tmpHome) - defer os.Setenv("HOME", origHome) + t.Setenv("HOME", tmpHome)This pattern appears at lines 69–71, 106–108, 140–142, and 176–178.
test/emulator.ts (3)
53-87:mobilecli,createCoverageDirectory, and verification helpers are duplicated acrossemulator.ts,simulator.ts, andios-device.ts.The
mobilecliwrapper (including coverage directory setup, error logging),takeScreenshot,verifyScreenshotFileWasCreated,verifyScreenshotFileHasValidContent, and several other helpers are nearly identical across all three test files. This violates DRY and means bug fixes or changes (e.g., timeout adjustments, logging format) must be replicated in three places.Consider extracting these into a shared
test/helpers.tsmodule.
108-117: Inconsistent typing:getDeviceInforeturnsanyhere butDeviceInfoResponseinsimulator.ts.In
test/simulator.ts(line 279),getDeviceInforeturnsDeviceInfoResponse, while here it returnsany. Similarly,verifyDeviceInfotakesanyinstead ofDeviceInfoResponse. SinceDeviceInfoResponseis already defined intest/types.ts, use it here for consistency and type safety.♻️ Suggested fix
-function getDeviceInfo(deviceId: string): any { +function getDeviceInfo(deviceId: string): DeviceInfoResponse { return mobilecli(['device', 'info', '--device', deviceId]); } -function verifyDeviceInfo(info: any, deviceId: string): void { +function verifyDeviceInfo(info: DeviceInfoResponse, deviceId: string): void { expect(info.data.device.id).to.equal(deviceId);
29-37:asyncon test callbacks is unnecessary when there are noawaitexpressions.The
should take screenshottest (line 29) and others are markedasyncbut contain noawaitcalls. While harmless, this is misleading about the test's nature.test/simulator.ts (2)
6-10: Duplicatefsimport:mkdirSyncis already available via* as fs.Line 4 imports
* as fsand line 10 imports{mkdirSync} from "fs". The named import is redundant sincefs.mkdirSyncis already accessible.♻️ Suggested fix
import * as fs from 'fs'; import { findSimulatorByName, bootSimulator, waitForSimulatorReady, } from './simctl'; -import {mkdirSync} from "fs";Then update usage on line 210:
- mkdirSync(dir, {recursive: true}); + fs.mkdirSync(dir, {recursive: true});
161-195: Lifecycle test does shutdown → boot → reboot → boot in a single test case.This is a very long sequential test with many sleep waits totaling ~30+ seconds of idle time. If any intermediate step fails, it's hard to diagnose which operation broke. Also, if the test fails mid-way (e.g., after shutdown), the simulator is left in an unknown state for subsequent tests.
Consider adding an
afterEachorafterhook to ensure the simulator is re-booted if the test fails, preventing cascading failures in the test suite.server/server.go (1)
73-74: Considerchan struct{}instead ofchan os.SignalforshutdownChan.The receiver (
<-shutdownChanon line 271) never inspects the signal value — it only cares that something was sent. Usingchan struct{}is more idiomatic Go for pure signaling channels, and avoids the misleading import dependency onos.Signal/syscall.SIGTERMin the handler.♻️ Suggested refactor
-var shutdownChan chan os.Signal +var shutdownChan chan struct{}Update initialization (line 200):
- shutdownChan = make(chan os.Signal, 1) + shutdownChan = make(chan struct{}, 1)Update sender (line 934):
- case shutdownChan <- syscall.SIGTERM: + case shutdownChan <- struct{}{}:test/ios-device.ts (3)
4-5: Duplicatefsimport —mkdirSyncalready available via* as fs.Same issue as in
simulator.ts: line 4 imports* as fs, makingfs.mkdirSyncavailable. The named import on line 5 is redundant.♻️ Suggested fix
import * as fs from 'fs'; -import {mkdirSync} from "fs";Then on line 180:
- mkdirSync(dir, {recursive: true}); + fs.mkdirSync(dir, {recursive: true});
178-212:mobilecliwrapper and helpers are copy-pasted fromemulator.tsandsimulator.ts.As noted in the emulator.ts review, this entire block (coverage directory creation, CLI execution, error logging) is duplicated three times. Extracting to a shared module would eliminate ~60 lines of duplication per file.
289-329:dumpUI,findElementByName, andverifyElementExistsare identical tosimulator.ts.These UI helper functions (lines 289–329) are character-for-character identical to their counterparts in
simulator.ts(lines 320–368). This is another strong candidate for extraction into the shared helpers module.
| e2e_test: | ||
| runs-on: [self-hosted, macOS, ARM64] | ||
| timeout-minutes: 15 | ||
| needs: [build_on_macos] | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Enable KVM | ||
| run: | | ||
| echo 'KERNEL=="kvm", GROUP="kvm", MODE="0666", OPTIONS+="static_node=kvm"' | sudo tee /etc/udev/rules.d/99-kvm4all.rules | ||
| sudo udevadm control --reload-rules | ||
| sudo udevadm trigger --name-match=kvm | ||
|
|
||
| - name: Setup JDK 17 | ||
| uses: actions/setup-java@v4 | ||
| with: | ||
| java-version: '17' | ||
| distribution: 'adopt' | ||
|
|
||
| - name: Set up Android SDK | ||
| uses: android-actions/setup-android@v3 | ||
| with: | ||
| cmdline-tools-version: 11479570 | ||
|
|
||
| - name: Install Android SDK components | ||
| run: | | ||
| yes | sdkmanager --licenses | ||
| sdkmanager "platform-tools" "platforms;android-${{ matrix.api_level }}" | ||
| sdkmanager "system-images;android-${{ matrix.api_level }};google_apis;x86_64" | ||
| sdkmanager "emulator" | ||
|
|
||
| - name: List AVD directory | ||
| run: | | ||
| ls -la ~/.android/avd || echo "AVD directory does not exist" | ||
|
|
||
| - name: Download linux build | ||
| - name: Download macos build | ||
| uses: actions/download-artifact@v4 | ||
| with: | ||
| name: linux-build | ||
| name: macos-build | ||
| path: . | ||
|
|
||
| - name: Make mobilecli executable | ||
| run: | | ||
| mv mobilecli-linux-amd64 mobilecli | ||
| mv mobilecli-darwin-arm64 mobilecli | ||
| chmod +x mobilecli | ||
|
|
||
| - name: npm install | ||
| - name: Run E2E tests | ||
| run: | | ||
| cd test | ||
| npm install --ignore-scripts | ||
|
|
||
| - name: run tests | ||
| uses: reactivecircus/android-emulator-runner@v2 | ||
| with: | ||
| api-level: ${{ matrix.api_level }} | ||
| arch: x86_64 | ||
| force-avd-creation: false | ||
| emulator-options: -no-snapshot-save -no-window -gpu swiftshader_indirect -noaudio -no-boot-anim -camera-back none | ||
| disable-animations: true | ||
| target: google_apis | ||
| working-directory: test | ||
| script: npm run test -- --grep "Android API ${{ matrix.api_level }}" | ||
|
|
||
| # - name: Run tests for Android API ${{ matrix.api_level }} | ||
| # run: | | ||
| # cd test | ||
| # npm install --ignore-scripts | ||
| # npm run test -- --grep "Android API ${{ matrix.api_level }}" | ||
| npm test |
There was a problem hiding this comment.
e2e_test is not a dependency of publish — is this intentional?
The publish job (line 234) depends on [build_on_windows, build_on_linux, build_on_macos, server_test] but does not include e2e_test. This means a release can be published even when E2E tests fail. If this is intentional (e.g., because self-hosted runner availability is unreliable), consider adding a comment to document the decision. Otherwise, add e2e_test to the needs list.
🤖 Prompt for AI Agents
In @.github/workflows/build.yml around lines 203 - 225, The publish job
currently does not depend on the e2e_test job, allowing releases to proceed even
if E2E fails; update the publish job's needs list (reference the publish job and
the e2e_test job names) to include e2e_test so publish depends on it, or if
omission is intentional add an inline comment next to the publish job explaining
why e2e_test is excluded (e.g., flaky self-hosted runner) to document the
decision.
| func TestAndroidDevice_PressButton_KeyMap(t *testing.T) { | ||
| validKeys := []string{ | ||
| "HOME", "BACK", "VOLUME_UP", "VOLUME_DOWN", "ENTER", | ||
| "DPAD_CENTER", "DPAD_UP", "DPAD_DOWN", "DPAD_LEFT", "DPAD_RIGHT", | ||
| "BACKSPACE", "APP_SWITCH", "POWER", | ||
| } | ||
|
|
||
| // just verify the key map has entries for all expected keys | ||
| keyMap := map[string]string{ | ||
| "HOME": "KEYCODE_HOME", | ||
| "BACK": "KEYCODE_BACK", | ||
| "VOLUME_UP": "KEYCODE_VOLUME_UP", | ||
| "VOLUME_DOWN": "KEYCODE_VOLUME_DOWN", | ||
| "ENTER": "KEYCODE_ENTER", | ||
| "DPAD_CENTER": "KEYCODE_DPAD_CENTER", | ||
| "DPAD_UP": "KEYCODE_DPAD_UP", | ||
| "DPAD_DOWN": "KEYCODE_DPAD_DOWN", | ||
| "DPAD_LEFT": "KEYCODE_DPAD_LEFT", | ||
| "DPAD_RIGHT": "KEYCODE_DPAD_RIGHT", | ||
| "BACKSPACE": "KEYCODE_DEL", | ||
| "APP_SWITCH": "KEYCODE_APP_SWITCH", | ||
| "POWER": "KEYCODE_POWER", | ||
| } | ||
|
|
||
| for _, key := range validKeys { | ||
| if _, ok := keyMap[key]; !ok { | ||
| t.Errorf("key %q missing from keyMap", key) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
PressButton_KeyMap test is self-referential — it doesn't validate production code.
This test defines its own keyMap locally and asserts against it. It never references the actual key map used by PressButton in android.go, so it provides zero coverage of the real mapping logic. If someone changes the production key map (e.g., typos a KEYCODE), this test would still pass.
To be useful, this test should either:
- Call
d.PressButton(key)with a mock/stub adb and verify the correct KEYCODE is passed, or - Directly reference the production key map variable (if exported or accessible within the package).
🤖 Prompt for AI Agents
In `@devices/android_test.go` around lines 290 - 319, The test
TestAndroidDevice_PressButton_KeyMap is self-referential and validates a locally
defined keyMap instead of the real mapping used by PressButton in android.go;
update the test to exercise production code by either (A) using the device's
actual key map variable (make it package-accessible or export it) and assert
entries match validKeys, or (B) preferibly instantiate an AndroidDevice, inject
a mock/stub ADB client, call d.PressButton(key) for each validKeys and assert
the mock received the expected KEYCODE string (verify calls to the ADB send/exec
method), targeting the PressButton method and the production key map (the key
map variable used by PressButton) to ensure real mapping is tested.
| "device.apps.terminate": handleAppsTerminate, | ||
| "device.apps.list": handleAppsList, | ||
| "device.apps.foreground": handleAppsForeground, | ||
| "server.shutdown": handleServerShutdown, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check for bind address configuration and defaults
rg -n "defaultServerAddress|0\.0\.0\.0|bind.*addr|ListenAndServe|Listen" --type=go -C2Repository: mobile-next/mobilecli
Length of output: 5742
🏁 Script executed:
# Check for any authentication middleware or authorization checks
rg -n "auth|Auth|permission|Permission|Authorization|middleware" --type=go -i -C1 | head -100Repository: mobile-next/mobilecli
Length of output: 1327
🏁 Script executed:
# Look at CORS configuration
rg -n "Access-Control|CORS|cors" --type=go -i -C2Repository: mobile-next/mobilecli
Length of output: 5310
🏁 Script executed:
# Check the handleServerShutdown implementation
rg -n "handleServerShutdown|func.*Shutdown" --type=go -A5Repository: mobile-next/mobilecli
Length of output: 6831
🏁 Script executed:
# Check main server startup and configuration
git ls-files | grep -E "(main|server|config)" | head -20Repository: mobile-next/mobilecli
Length of output: 215
🏁 Script executed:
# Check handleServerShutdown implementation more completely
rg -n "handleServerShutdown" --type=go -B2 -A10Repository: mobile-next/mobilecli
Length of output: 1478
🏁 Script executed:
# Check if there are any authorization checks in the dispatch or RPC handler
rg -n "handleJSONRPC|Dispatch|execute.*method" --type=go -A15 | head -80Repository: mobile-next/mobilecli
Length of output: 1660
🏁 Script executed:
# Check if Authorization header is actually validated anywhere
rg -n "Authorization|auth.*header|Bearer|token" --type=go -iRepository: mobile-next/mobilecli
Length of output: 165
🏁 Script executed:
# Look for any comments or documentation about security/auth
rg -n "security|auth|permission|unauthenticated|restricted" --type=go -i | head -30Repository: mobile-next/mobilecli
Length of output: 620
🏁 Script executed:
# Check for any middleware that might do authorization
rg -n "middleware|handler.*auth|guard|verify|validate.*auth" --type=go -i -A3Repository: mobile-next/mobilecli
Length of output: 2716
🏁 Script executed:
# Check the full handleJSONRPC to see if there's auth inside
cat -n server/server.go | sed -n '277,330p'Repository: mobile-next/mobilecli
Length of output: 2010
🏁 Script executed:
# Check if Authorization is used anywhere after being set in CORS headers
rg -n "r\.Header.*Authorization|extractToken|getBearerToken" --type=go -iRepository: mobile-next/mobilecli
Length of output: 47
server.shutdown is unauthenticated and can be invoked by any network peer when exposed with CORS.
The default configuration binds to localhost:12000 and is safe. However, if the server is run with --listen 0.0.0.0:<port> --cors, the endpoint becomes vulnerable to denial-of-service: any peer can invoke server.shutdown to terminate the server since no authentication layer exists.
The CORS middleware advertises Authorization header support but never validates it. Consider either:
- Documenting this risk for local/development-only use, or
- Adding an authorization check to
handleServerShutdown(and other privileged operations likedevice.reboot), or - Restricting this endpoint to localhost addresses
🤖 Prompt for AI Agents
In `@server/dispatch.go` at line 36, The server.shutdown endpoint is currently
unauthenticated (invoked via handleServerShutdown) and can be abused when
CORS/external listen is enabled; update handleServerShutdown to enforce
authorization or restrict callers: either validate an Authorization header
(e.g., check a bearer token/verify JWT or shared secret from request headers
against server config) before performing shutdown, or verify the remote address
is a loopback/localhost address and reject non-local requests; also apply the
same protection to other privileged handlers (e.g., device.reboot) so privileged
operations require the same auth or localhost-only check.
| srcInfo, _ := os.Stat(srcPath) | ||
| dstInfo, _ := os.Stat(dstPath) |
There was a problem hiding this comment.
Ignoring os.Stat errors could cause nil-pointer panic on failure.
If either os.Stat call fails unexpectedly, srcInfo or dstInfo will be nil, causing a panic on line 50. Use t.Fatal on error.
🔧 Proposed fix
- srcInfo, _ := os.Stat(srcPath)
- dstInfo, _ := os.Stat(dstPath)
+ srcInfo, err := os.Stat(srcPath)
+ if err != nil {
+ t.Fatalf("failed to stat source: %v", err)
+ }
+ dstInfo, err := os.Stat(dstPath)
+ if err != nil {
+ t.Fatalf("failed to stat dest: %v", err)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| srcInfo, _ := os.Stat(srcPath) | |
| dstInfo, _ := os.Stat(dstPath) | |
| srcInfo, err := os.Stat(srcPath) | |
| if err != nil { | |
| t.Fatalf("failed to stat source: %v", err) | |
| } | |
| dstInfo, err := os.Stat(dstPath) | |
| if err != nil { | |
| t.Fatalf("failed to stat dest: %v", err) | |
| } |
🤖 Prompt for AI Agents
In `@utils/file_test.go` around lines 47 - 48, The test currently ignores errors
from os.Stat when populating srcInfo and dstInfo which can be nil and cause a
panic; update the calls in utils/file_test.go so you check the returned error
from os.Stat for both srcPath and dstPath and call t.Fatal (with the error) if
either returns non-nil, ensuring srcInfo and dstInfo are valid before they are
used on line 50.
1 participant
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests