fix(os-service): use pathToFileURL for cross-platform log folder path#3030
Conversation
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
|
Reviews (1): Last reviewed commit: "fix(os-service): use pathToFileURL for c..." | Re-trigger Greptile |
| expect(urlLauncher.launch).toHaveBeenCalledWith( | ||
| expect.stringMatching(/^file:\/\//), | ||
| ); |
There was a problem hiding this comment.
The assertion only checks the
file:// protocol prefix, but the mock already sets logFolderPath: "/logs" — a deterministic value. On any platform pathToFileURL("/logs").href returns "file:///logs", so the test could assert the complete URL and thereby also verify that the correct path was forwarded to pathToFileURL. As written, the test would pass even if showLogFolder called pathToFileURL("") or any other path.
| expect(urlLauncher.launch).toHaveBeenCalledWith( | |
| expect.stringMatching(/^file:\/\//), | |
| ); | |
| expect(urlLauncher.launch).toHaveBeenCalledWith("file:///logs"); |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
844aef4 to
cf81f7e
Compare
…, add test Generated-By: PostHog Code Task-Id: 422bdee4-0988-45ff-b47e-4337650d40b1
aa88fce to
7efc24d
Compare
|
Merged into #3031. |
7efc24d
into
posthog-code/command-menu-developer-section
Summary
Follow-up to #3005. Fixes
showLogFolderto produce a valid file URI on all platforms.file://${path}breaks on Windows (C:\...→file://C:\..., invalid)pathToFileURL(path).href(Node built-in) handles this correctly on all platformsfile://URLCreated with PostHog Code