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

http.batch() should not accept more than one argument #1289

Closed
na-- opened this issue Dec 27, 2019 · 4 comments · Fixed by #2585
Closed

http.batch() should not accept more than one argument #1289

na-- opened this issue Dec 27, 2019 · 4 comments · Fixed by #2585
Assignees
Labels
good first issue lower prio new-http issues that would require (or benefit from) a new HTTP API ux
Milestone

Comments

@na--
Copy link
Member

na-- commented Dec 27, 2019

It took me a while to understand why this code didn't work 🤕 :

// ...
let responses = http.batch(
    ["GET", "https://test.loadimpact.com/"],
    ["GET", "https://test.loadimpact.com/", null, { responseType: "text" }],
);
// ...

The only warning I saw was

WARN[0000] Request Failed                                error="Get GET: unsupported protocol scheme \"\""

Since http.batch() doesn't accept more than one argument, we should probably emit a warning with a more sensible message (or maybe even throw an error) when users pass us more...

@na-- na-- added the new-http issues that would require (or benefit from) a new HTTP API label Oct 18, 2021
@vanshaj
Copy link
Contributor

vanshaj commented Jul 1, 2022

Hello @na--

I have started working on this issue and was able to find the fix. If no one else is working on it , can you please assign this issue to me. I will resolve it.

Regards
Vanshaj Dhar

@vanshaj
Copy link
Contributor

vanshaj commented Jul 1, 2022

Hello @na--

I would also wanted to add one point that there are some unit tests that contains the same scenario as yours but contains invalid urls, need to update those test scenarios also.
kindly update me on the same

Regards
Vanshaj

@na--
Copy link
Member Author

na-- commented Jul 4, 2022

there are some unit tests that contains the same scenario as yours but contains invalid urls

can you share a link to that code?

@vanshaj
Copy link
Contributor

vanshaj commented Jul 4, 2022

Hello @na--
Sorry I didn't look at the tests properly, the tests doesnot contain the same scenarios . All the tests have only one array of items not multiple arrays.

I will add the unit test for the same. Along with validation for previous tests also.
Thanks for the assignment

Regards
Vanshaj

vanshaj added a commit to vanshaj/k6 that referenced this issue Jul 4, 2022
@na-- na-- added this to the v0.40.0 milestone Jul 4, 2022
vanshaj added a commit to vanshaj/k6 that referenced this issue Jul 4, 2022
vanshaj added a commit to vanshaj/k6 that referenced this issue Jul 4, 2022
vanshaj added a commit to vanshaj/k6 that referenced this issue Jul 5, 2022
vanshaj added a commit to vanshaj/k6 that referenced this issue Jul 5, 2022
vanshaj added a commit to vanshaj/k6 that referenced this issue Jul 6, 2022
vanshaj added a commit to vanshaj/k6 that referenced this issue Jul 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue lower prio new-http issues that would require (or benefit from) a new HTTP API ux
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants