Replace hand-rolled file-URL encoder with url::Url::from_file_path#44
Closed
kasnder wants to merge 2 commits into
Closed
Replace hand-rolled file-URL encoder with url::Url::from_file_path#44kasnder wants to merge 2 commits into
kasnder wants to merge 2 commits into
Conversation
The test fed path_to_file_url a "ReDD Blocker.app" path but asserted the pre-rename "ReDD%20Block.app" URL, so it has failed since the product rename. Use a product-agnostic fixture path instead — the test is about percent-encoding, not the bundle name, so a future rename can't break it again. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
path_to_file_url + pct_encode_path_segment reimplemented file-URL percent-encoding by hand, guarded by a single test that itself proved brittle (it hardcoded the pre-rename bundle name). Delegate to the url crate instead — already in the dependency tree via Tauri, so this adds no new compiled code — and surface its relative-path rejection as an Option handled at the resolve_block_page_url call site. Tests now cover spaces, non-ASCII, and relative-path rejection with a product-agnostic fixture, so a future app rename can't break them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
kasnder
changed the title
Collaborator
Author
|
Closing — folding the one-line test fix into #43 instead, which is where the failing test shows up. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
web_automation::tests::file_url_encodes_spaceshas been failing since the product rename: it fedpath_to_file_urla "ReDD Blocker.app" path but asserted the pre-renameReDD%20Block.appURL.The deeper issue wasn't the stale string — it's that
path_to_file_url+pct_encode_path_segmenthand-rolled file-URL percent-encoding, guarded by a single test that itself hardcoded the product name.Fix
url::Url::from_file_path— theurlcrate is already in the dependency tree via Tauri (2.5.8 in the lockfile), so declaring it as a direct dependency adds no new compiled code.path_to_file_urlnow returnsOption<String>(the crate rejects relative paths);resolve_block_page_urllogs and declines to start the watcher in that case, same as the existing missing-resource-dir path.Block-page detection is unaffected:
is_block_page_urlkeeps its normalization-tolerant fallback (contains("blocked/blocked.html")), and the base URL is generated once and used for both the AppleScriptset URLand the comparison, so both sides stay consistent. The query-stringpct_encode/pct_decodehelpers are untouched — they intentionally mirror the extension'sbackground.js.cargo test --lib: 30 passed, 0 failed.🤖 Generated with Claude Code