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

lib/netext/httpext: add support decompression of stacked compressed response #1125

Merged
merged 1 commit into from
Aug 29, 2019

Conversation

cuonglm
Copy link
Contributor

@cuonglm cuonglm commented Aug 26, 2019

k6 supports compression of request bodies with multiple layered algorithms but does not handle decompression body in the same manner. This PR adds support for decompression response body with multiple layered algorithms in the same manner.

Close #1108

@codecov-io
Copy link

codecov-io commented Aug 26, 2019

Codecov Report

Merging #1125 into master will increase coverage by 0.02%.
The diff coverage is 88.98%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1125      +/-   ##
==========================================
+ Coverage   73.37%   73.39%   +0.02%     
==========================================
  Files         141      142       +1     
  Lines       10305    10323      +18     
==========================================
+ Hits         7561     7577      +16     
- Misses       2302     2303       +1     
- Partials      442      443       +1
Impacted Files Coverage Δ
lib/netext/httpext/request.go 97.04% <ø> (+3.73%) ⬆️
lib/testutils/httpmultibin.go 95.48% <100%> (+0.61%) ⬆️
lib/netext/httpext/compression.go 87.25% <87.25%> (ø)
core/engine.go 92.99% <0%> (-0.94%) ⬇️

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 f2dff90...ee081f1. Read the comment docs.

imiric
imiric previously approved these changes Aug 27, 2019
Copy link
Contributor

@imiric imiric left a comment

Choose a reason for hiding this comment

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

LGTM, nice work! Just left a minor suggestion.

lib/testutils/httpmultibin.go Show resolved Hide resolved
Copy link
Contributor

@mstoykov mstoykov left a comment

Choose a reason for hiding this comment

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

Seems okay, but there are a couple of things that I would prefer if are done differently:

  1. the if !assert .. is IMO an antipattern scattered everywhere in k6 that I remove when I see it and would prefer if we don't add it back ;)
  2. the reversing of the contentEncodings could be done either in a way that is more easy to understand what happens if not faster (as speed in this case is probably not important) or you can just walk it with a real for backwards which seems to me like the even better solution ;)

lib/netext/httpext/compression.go Outdated Show resolved Hide resolved
lib/testutils/httpmultibin.go Outdated Show resolved Hide resolved
lib/netext/httpext/request.go Show resolved Hide resolved
Copy link
Member

@na-- na-- left a comment

Choose a reason for hiding this comment

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

LGTM now

Copy link
Contributor

@mstoykov mstoykov left a comment

Choose a reason for hiding this comment

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

LGTM

@cuonglm cuonglm merged commit 14c8177 into master Aug 29, 2019
@cuonglm cuonglm deleted the feature/issue-1108 branch August 29, 2019 10:00
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.

Support decompression of stacked compressed responses
6 participants