go-resty / resty

Simple HTTP and REST client library for Go
MIT License
9.89k stars 696 forks source link

Improve parseRequestHeader function and add the unit tests and benchmarks #712

Closed SVilgelm closed 11 months ago

SVilgelm commented 11 months ago

Original benchmarks:

% go test -benchmem -bench=. -run=^Benchmark
goos: darwin
goarch: amd64
pkg: github.com/go-resty/resty/v2
cpu: Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
Benchmark_parseRequestHeader-16    1168611    1042 ns/op    496 B/op    8 allocs/op

After the improvements:

% go test -benchmem -bench=. -run=^Benchmark
goos: darwin
goarch: amd64
pkg: github.com/go-resty/resty/v2
cpu: Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
Benchmark_parseRequestHeader-16    7956481    155.6 ns/op    0 B/op    0 allocs/op

Improved by modifying the original request headers instead of creating a new object

codecov[bot] commented 11 months ago

Codecov Report

All modified lines are covered by tests :white_check_mark:

Comparison is base (b852413) 96.34% compared to head (60e3194) 96.33%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #712 +/- ## ========================================== - Coverage 96.34% 96.33% -0.02% ========================================== Files 12 12 Lines 1615 1610 -5 ========================================== - Hits 1556 1551 -5 Misses 37 37 Partials 22 22 ``` | [Files](https://app.codecov.io/gh/go-resty/resty/pull/712?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-resty) | Coverage Δ | | |---|---|---| | [middleware.go](https://app.codecov.io/gh/go-resty/resty/pull/712?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-resty#diff-bWlkZGxld2FyZS5nbw==) | `92.85% <100.00%> (-0.12%)` | :arrow_down: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

jeevatkm commented 11 months ago

Thanks @SVilgelm, this PR is a comparatively small changeset than URL params PR, I will let you know!

SVilgelm commented 11 months ago

Yep, that's why I created it as separated