Fixes path order dependent response when a nil handler is defined for a route

[rkilingr]

Fixes #515

Summary of Changes

1. Checks route handler != nil condition only for setting the matched handler, not for clearing Method mismatch error
2. Adds test-cases to check order dependent response if request handler for the path is nil

[elithrar]

Break these into subtests - e.g.

t.Run("Test handler ordering", func(t *testing.T) { 
   // test 
})

t.Run("Test reversed handler ordering", func(t *testing.T) { 
   // test 
})

I would also add additional test cases:

1. Test ordering where handlers have the same method & path, but a different handler (expected: the first handler added is the one executed)
2. Same path, different methods (no path variables)
3. Same as Host() with a port fails to match when the host is supplied in the HTTP Host: header #2, but with middleware

[rkilingr]

Have made the listed changes

[elithrar]

This should be before the paths are added.

[elithrar]

Make it a subtest (as above) and call newRequest again to separate it.

[elithrar]

@rkilingr - did you still want to complete this?

[elithrar]

@rkilingr - just following up again if you're still keen on completing this?

[rkilingr]

@rkilingr - just following up again if you're still keen on completing this?

Apologies for the huge delay. Was not active last year, will be updating the PR

[amustaque97]

Hi @rkilingr - quick follow-up - are you interested in getting this merged? Will do a review again if existing comments are addressed.

[codecov]

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 78.19%. Comparing base (85123bf) to head (9576551).
⚠️ Report is 19 commits behind head on main.

Files with missing lines	Patch %	Lines
route.go	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #517      +/-   ##
==========================================
+ Coverage   78.01%   78.19%   +0.17%     
==========================================
  Files           5        5              
  Lines         887      885       -2     
==========================================
  Hits          692      692              
+ Misses        140      138       -2     
  Partials       55       55              

☔ View full report in Codecov by Sentry.
📢 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.

[pcfreak30]

@coreydaley @jackgris @rkilingr this change really needs to get merged in tbh. im debugging my routing and the if match.MatchErr != nil && r.handler != nil { logic seems to do some really funky things when you have routes being tested before a valid one.

[apoorvajagtap]

Hello @pcfreak30 Thanks for the work!
Corey is no longer active on the project, hence the pings went unaddressed. The maintainer details are available here.

I have just started looking into this issue. Would you be able to do a rebase to eliminate the conflicts?

[pcfreak30]

Hello @pcfreak30 Thanks for the work! Corey is no longer active on the project, hence the pings went unaddressed. The maintainer details are available here.

I have just started looking into this issue. Would you be able to do a rebase to eliminate the conflicts?

Um, this isn't my PR so im not sure what you mean.... Though I do want to see this bug fixed :P.

[apoorvajagtap]

@pcfreak30 my bad. I mixed up two different communications.
@rkilingr would you be able to rebase this?