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

Open()'ing binary files and multipart uploads #524

Merged
merged 7 commits into from Mar 14, 2018

Conversation

robingustafsson
Copy link
Member

This a continuation of #420 (also overriding #370).

It implements support for open()ing binary files and multipart uploads with the following API:

import http from "k6/http";

let binFile = open("/path/to/file.bin", "b");

export default function() {
    var data = {
        field: "this is a standard form field",
        file: http.file(binFile, "test.bin")
    };
    var res = http.post("https://example.com/upload", data);
}

@robingustafsson
Copy link
Member Author

@spicykoala Here's the new PR :) I feel it's more or less ready to be merged but if you can give me feedback and/or test this PR that'd be great.

@na-- If you have any feedback on this PR I'd appreciate it as well.

ContentType string
}

var quoteEscaper = strings.NewReplacer("\\", "\\\\", `"`, "\\\"")
Copy link
Member

Choose a reason for hiding this comment

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

This escaping seems a bit strange. I think it's equivalent to strings.NewReplacer(`\`, `\\`, `"`, `\"`), but much harder to understand because of the mixed quotes. Also probably deserves a simple unit test just in case.

Copy link
Member

Choose a reason for hiding this comment

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

Taking my words back since this is basically copied from https://golang.org/src/mime/multipart/writer.go?s=2899:3037#L126

fname, ct := fmt.Sprintf("%d", time.Now().UnixNano()), "application/octet-stream"

if len(args) > 0 {
fname = escapeQuotes(args[0])
Copy link
Member

Choose a reason for hiding this comment

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

These strings should probably be escaped in the context they are used (line 124-125), not in the FileData struct "constructor"

@codecov-io
Copy link

Codecov Report

Merging #524 into master will decrease coverage by 0.2%.
The diff coverage is 39.39%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #524      +/-   ##
==========================================
- Coverage   62.86%   62.66%   -0.21%     
==========================================
  Files          95       94       -1     
  Lines        6962     6835     -127     
==========================================
- Hits         4377     4283      -94     
+ Misses       2339     2302      -37     
- Partials      246      250       +4
Impacted Files Coverage Δ
js/modules/k6/http/file.go 0% <0%> (ø)
js/initcontext.go 100% <100%> (ø) ⬆️
js/modules/k6/crypto/crypto.go 97.29% <100%> (ø) ⬆️
js/modules/k6/http/http_request.go 81.62% <24.32%> (-7.38%) ⬇️
lib/netext/dialer.go 42.42% <0%> (-31.11%) ⬇️
stats/thresholds.go 79.41% <0%> (-13.93%) ⬇️
cmd/common.go 5.71% <0%> (-8.58%) ⬇️
cmd/run.go 2.4% <0%> (-3.9%) ⬇️
cmd/archive.go 12.5% <0%> (-3.72%) ⬇️
cmd/inspect.go 9.67% <0%> (-3.15%) ⬇️
... and 17 more

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 e10979a...06a22ca. Read the comment docs.

@robingustafsson robingustafsson merged commit b5cf313 into master Mar 14, 2018
@na-- na-- deleted the feature/multipart-binary branch March 16, 2018 09:53
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.

None yet

4 participants