Skip to content

test(inertia): add test for invalid url input#2007

Open
nkfr26 wants to merge 1 commit into
honojs:mainfrom
nkfr26:test/inertia-page-url-non-get
Open

test(inertia): add test for invalid url input#2007
nkfr26 wants to merge 1 commit into
honojs:mainfrom
nkfr26:test/inertia-page-url-non-get

Conversation

@nkfr26

@nkfr26 nkfr26 commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

The corresponding PR: #1991

I've added tests, as the tests for the implementation added later were missing.

Also, please let me know if I need to add myself to the Authors section in the README — I'll take care of it. I'm also planning to continue contributing (with at least two more feature proposals to come, btw).

The author should do the following, if applicable

  • Add tests
  • Run tests
  • yarn changeset at the top of this repo and push the changeset
  • Follow the contribution guide

@changeset-bot

changeset-bot Bot commented Jun 27, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: b0bed91

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@codecov

codecov Bot commented Jun 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.06%. Comparing base (48bd7a0) to head (b0bed91).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2007      +/-   ##
==========================================
+ Coverage   92.04%   92.06%   +0.02%     
==========================================
  Files         115      115              
  Lines        4073     4073              
  Branches     1064     1064              
==========================================
+ Hits         3749     3750       +1     
+ Misses        288      287       -1     
  Partials       36       36              
Flag Coverage Δ
inertia 100.00% <ø> (+0.65%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@yusukebe

Copy link
Copy Markdown
Member

Hey @ashunar0, can you review this?

@nkfr26 @ashunar0

Also, please let me know if I need to add myself to the Authors section in the README

Maybe my explanation is not enough. In this middleware repo, "Author" means the author has responsibility for the middleware and must be a reviewer for it. This means the author is stronger than just a contributor. For now, I think we should keep @nkfr26 as a contributor. If @nkfr26 really wants to be an author, though, we can add @nkfr26 to the README.

@ashunar0

Copy link
Copy Markdown
Contributor

@nkfr26 Thanks for backfilling the tests for the catch branch!

The first test (Referer: 'invalid-url') looks good — new URL('invalid-url') throws, so it exercises the catch branch as intended 👍

However, the second test (options.url: '#') doesn't actually hit the catch branch. new URL('#', 'http://localhost/users') does not throw — the WHATWG URL parser resolves it against the base to http://localhost/users# (pathname /users, empty search). So body.url ends up as /users via the try success path, not the fallback. The comment // '#' is unusable as a URL because it's just a URI fragment is therefore not quite accurate.

To actually exercise the catch branch, an absolute URL with malformed syntax would work better. For example:

app.post('/users', (c) => c.render('Users/New', {}, { url: 'http://[bad' }))

new URL('http://[bad', c.req.url) throws, so this would reliably route through the fallback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants