Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #2786 #2796

Merged
merged 2 commits into from Jul 26, 2021
Merged

fix #2786 #2796

merged 2 commits into from Jul 26, 2021

Conversation

qm012
Copy link
Contributor

@qm012 qm012 commented Jul 23, 2021

  • More unit tests
  • Change the route matching rule to match multi-level routes
  • Fixed 404 not found when each level of route is greater than or less than the matched path and has the same prefix
  • Error with argument being intercepted

@appleboy @thinkerou I will provide the test report tomorrow, thanks.

@codecov
Copy link

codecov bot commented Jul 23, 2021

Codecov Report

Merging #2796 (4c4e5d6) into master (d4ca9a0) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2796      +/-   ##
==========================================
- Coverage   98.71%   98.71%   -0.01%     
==========================================
  Files          41       41              
  Lines        2097     2094       -3     
==========================================
- Hits         2070     2067       -3     
  Misses         15       15              
  Partials       12       12              
Impacted Files Coverage Δ
tree.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d4ca9a0...4c4e5d6. Read the comment docs.

@appleboy appleboy added the bug label Jul 24, 2021
@appleboy appleboy added this to the v1.8 milestone Jul 24, 2021
@appleboy appleboy linked an issue Jul 24, 2021 that may be closed by this pull request
@qm012
Copy link
Contributor Author

qm012 commented Jul 24, 2021

hi @appleboy @thinkerou I updated the test report. thanks.

The latest benchmarks:

branch benchmark
master https://travis-ci.com/github/qm012/go-http-routing-benchmark/builds/233564098
this PR https://travis-ci.com/github/qm012/go-http-routing-benchmark/builds/233564278

The benchstat report:

$ benchstat old.txt new.txt
name              old time/op    new time/op    delta
Gin_Param           61.4ns ± 1%    60.3ns ± 4%  -1.69%  (p=0.006 n=9+9)
Gin_Param5           117ns ± 1%     116ns ± 1%  -1.43%  (p=0.000 n=9+9)
Gin_Param20          305ns ± 2%     303ns ± 0%  -0.67%  (p=0.027 n=10+10)
Gin_ParamWrite       114ns ± 2%     114ns ± 1%    ~     (p=0.346 n=9+9)
Gin_GithubStatic    80.2ns ± 1%    79.0ns ± 2%  -1.55%  (p=0.029 n=10+10)
Gin_GithubParam      139ns ± 1%     133ns ± 2%  -4.12%  (p=0.000 n=10+10)
Gin_GithubAll       27.7µs ± 1%    26.2µs ± 0%  -5.45%  (p=0.000 n=10+10)
Gin_GPlusStatic     63.2ns ± 3%    60.6ns ± 0%  -4.20%  (p=0.000 n=10+7)
Gin_GPlusParam      94.3ns ± 3%    89.6ns ± 1%  -4.98%  (p=0.000 n=10+10)
Gin_GPlus2Params     119ns ± 2%     115ns ± 0%  -3.59%  (p=0.000 n=10+10)
Gin_GPlusAll        1.22µs ± 3%    1.15µs ± 1%  -6.14%  (p=0.000 n=10+10)
Gin_ParseStatic     59.7ns ± 1%    61.6ns ± 1%  +3.18%  (p=0.000 n=9+10)
Gin_ParseParam      68.6ns ± 3%    68.7ns ± 1%    ~     (p=0.661 n=10+9)
Gin_Parse2Params    83.4ns ± 0%    84.7ns ± 1%  +1.52%  (p=0.000 n=8+10)
Gin_ParseAll        1.99µs ± 2%    1.96µs ± 0%  -1.77%  (p=0.000 n=10+9)
Gin_StaticAll       19.4µs ± 1%    18.5µs ± 1%  -4.82%  (p=0.000 n=10+9)

@appleboy appleboy requested review from appleboy and thinkerou Jul 25, 2021
@appleboy appleboy modified the milestones: v1.8, v1.x Jul 25, 2021
@appleboy
Copy link
Member

Waiting for @thinkerou approval.

Copy link
Member

@thinkerou thinkerou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@zihengCat
Copy link
Contributor

Well done!

@thinkerou thinkerou merged commit 0a55865 into gin-gonic:master Jul 26, 2021
18 of 19 checks passed
@qm012 qm012 deleted the develop-2786 branch Jul 26, 2021
@appleboy appleboy modified the milestones: v1.8, v1.7.3 Aug 3, 2021
appleboy pushed a commit that referenced this pull request Aug 15, 2021
* update match rule

* add comments
Bisstocuz pushed a commit to Bisstocuz/gin that referenced this pull request Nov 22, 2021
* update match rule

* add comments

(cherry picked from commit 0a55865)
Bisstocuz pushed a commit to Bisstocuz/gin that referenced this pull request Nov 22, 2021
* update match rule

* add comments

(cherry picked from commit 0a55865)
thinkerou pushed a commit that referenced this pull request Nov 23, 2021
* update match rule

* add comments
daheige pushed a commit to daheige/gin that referenced this pull request Apr 18, 2022
* update match rule

* add comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]mixed param and non-param paths
4 participants