test: improve http-client coverage#33633

KuthorX · 2020-05-29T05:34:18Z

Based on https://coverage.nodejs.org/coverage-d12d5ef3ef8e5d9f/lib/_http_client.js.html, I try to improve coverage to line 120 (http-client-input-function), 194-197 (http-client-insecure-http-parser-error)

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
commit message follows commit guidelines

BridgeAR · 2020-05-30T00:35:29Z

test/parallel/test-http-client-insecure-http-parser-error.js

+ assert.throws(() =>{
+ new ClientRequest({insecureHTTPParser: 'wrongValue'});
+ },{
+ code: /ERR_INVALID_ARG_TYPE/


Suggested change
code: /ERR_INVALID_ARG_TYPE/
code: 'ERR_INVALID_ARG_TYPE',
message: /insecureHTTPParser/

BridgeAR · 2020-05-30T00:39:26Z

test/parallel/test-http-client-input-function.js

+const ClientRequest = require('http').ClientRequest;
+
+{
+ const req = new ClientRequest(() =>{});


The test should definitely hit the mentioned code line but it would be great to write a test that actually verifies that the callback is also called and ideally, this test case would not cause any errors, since it's a legit code path.

@BridgeAR Here is a rewrote version, which can make cb be called, but it's a little longer than previous version, so I am not sure whether it's ok, could you help me to check?
'use strict';constcommon=require('../common');consthttp=require('http');constassert=require('assert');constClientRequest=require('http').ClientRequest;{constserver=http.createServer(common.mustCall((req,res)=>{res.writeHead(200);res.end('hello world');})).listen(80,'127.0.0.1');constreq=newClientRequest(common.mustCall((response)=>{letbody='';response.setEncoding('utf8');response.on('data',(chunk)=>{body+=chunk;});response.on('end',common.mustCall(()=>{assert.strictEqual(body,'hello world');server.close();}));}));req.end();}

BridgeAR · 2020-05-30T00:40:14Z

@KuthorX thank you very much for your contribution! This is already looking pretty good, just left two minor comments.

KuthorX · 2020-09-04T11:37:03Z

Could someone help me to review lastest commit? Any suggestion will be appreciated.

nodejs-github-bot · 2020-12-24T07:42:54Z

CI: https://ci.nodejs.org/job/node-test-pull-request/35061/

aduh95 · 2023-09-19T14:36:36Z

Ping @nodejs/http for reviews.

ShogunPanda · 2023-10-06T10:00:49Z

The changes look fine to me. There is only one test failing as you're using port 80. Can you use another port?

KuthorX · 2023-10-09T17:56:08Z

The changes look fine to me. There is only one test failing as you're using port 80. Can you use another port?

thanks, has changed to 8080

test/parallel/test-http-client-input-function.js

ShogunPanda

LGTM!

nodejs-github-bot · 2023-10-10T13:53:55Z

CI: https://ci.nodejs.org/job/node-test-pull-request/54662/

KuthorX · 2023-10-11T03:26:33Z

looks like some ci fail, maybe I should merge main branch? @ShogunPanda

mcollina

lgtm

nodejs-github-bot · 2023-10-11T08:14:53Z

CI: https://ci.nodejs.org/job/node-test-pull-request/54695/

nodejs-github-bot · 2023-10-29T12:41:29Z

CI: https://ci.nodejs.org/job/node-test-pull-request/55315/

nodejs-github-bot · 2023-10-30T11:08:16Z

CI: https://ci.nodejs.org/job/node-test-pull-request/55329/

nodejs-github-bot · 2023-12-04T09:10:58Z

CI: https://ci.nodejs.org/job/node-test-pull-request/56067/

nodejs-github-bot · 2023-12-18T11:23:16Z

CI: https://ci.nodejs.org/job/node-test-pull-request/56358/

nodejs-github-bot · 2023-12-19T08:06:57Z

CI: https://ci.nodejs.org/job/node-test-pull-request/56379/

nodejs-github-bot · 2023-12-19T09:55:23Z

Landed in c925039

PR-URL: nodejs#33633 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: James M Snell <[email protected]>

KuthorX · 2023-12-19T16:03:34Z

After 3 years, it merged, thank you guys 😌

PR-URL: #33633 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: James M Snell <[email protected]>

nodejs-github-bot added the test Issues and PRs related to the tests. label May 29, 2020

BridgeAR reviewed May 30, 2020
View reviewed changes

KuthorX requested a review from BridgeAR May 30, 2020 09:37

BridgeAR force-pushed the master branch 2 times, most recently from 8ae28ff to 2935f72Compare May 31, 2020 12:18

PoojaDurgad added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 24, 2020

github-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 24, 2020

guybedford force-pushed the master branch from dc5a5da to 8e46568Compare March 29, 2021 21:33

Trott force-pushed the main branch from 2d76238 to ca3ed36Compare November 12, 2022 01:49

aduh95 force-pushed the http-client-coverage branch from af3315a to d64869dCompare September 19, 2023 14:35

KuthorXand others added 4 commits September 19, 2023 16:57

http: do not override user-provided options object
915b724

fix code style by lint-js
2b0301f

modify error code&msg, make client cb be called
37d005a

fix test
289444e

aduh95 force-pushed the http-client-coverage branch from d64869d to 289444eCompare September 19, 2023 14:58

fix ci error, use different port
9eed9e1

mcollina requested changes Oct 10, 2023
View reviewed changes

test/parallel/test-http-client-input-function.js Outdated Show resolvedHide resolved

listen dynamic port
33e6591

KuthorX requested a review from mcollina October 10, 2023 09:39

ShogunPanda approved these changes Oct 10, 2023
View reviewed changes

ShogunPanda added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 10, 2023

github-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 10, 2023

github-actionsbot mentioned this pull request Oct 11, 2023
CI Reliability 2023-10-11 nodejs/reliability#690
Open
30 tasks

mcollina approved these changes Oct 11, 2023
View reviewed changes

jasnell approved these changes Oct 28, 2023
View reviewed changes

github-actionsbot mentioned this pull request Oct 30, 2023
CI Reliability 2023-10-30 nodejs/reliability#702
Open
25 tasks

This was referenced Oct 31, 2023
CI Reliability 2023-10-31 nodejs/reliability#703
Open
CI Reliability 2023-11-01 nodejs/reliability#704
Open
CI Reliability 2023-11-02 nodejs/reliability#705
Open

mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 4, 2023

github-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 4, 2023

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 19, 2023

nodejs-github-bot merged commit c925039 into nodejs:mainDec 19, 2023

RafaelGSS pushed a commit that referenced this pull request Jan 2, 2024
http: do not override user-provided options object
a325746
PR-URL: #33633 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: James M Snell <[email protected]>

RafaelGSS mentioned this pull request Jan 2, 2024
v21.6.0 proposal #51342
Merged

richardlau pushed a commit that referenced this pull request Mar 25, 2024
http: do not override user-provided options object
9062d30
PR-URL: #33633 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: James M Snell <[email protected]>

richardlau mentioned this pull request Mar 25, 2024
v20.12.0 proposal #52212
Merged

	code: /ERR_INVALID_ARG_TYPE/
	code: 'ERR_INVALID_ARG_TYPE',
	message: /insecureHTTPParser/

Uh oh!

test: improve http-client coverage#33633

test: improve http-client coverage #33633

Uh oh!

Conversation

KuthorX commented May 29, 2020

Checklist

Uh oh!

BridgeARMay 30, 2020

Choose a reason for hiding this comment

Uh oh!

BridgeARMay 30, 2020

Choose a reason for hiding this comment

Uh oh!

KuthorXMay 30, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BridgeAR commented May 30, 2020

Uh oh!

KuthorX commented Sep 4, 2020

Uh oh!

nodejs-github-bot commented Dec 24, 2020

Uh oh!

aduh95 commented Sep 19, 2023

Uh oh!

ShogunPanda commented Oct 6, 2023

Uh oh!

KuthorX commented Oct 9, 2023

Uh oh!

Uh oh!

ShogunPanda left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Oct 10, 2023

Uh oh!

KuthorX commented Oct 11, 2023

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Oct 11, 2023

Uh oh!

nodejs-github-bot commented Oct 29, 2023

Uh oh!

nodejs-github-bot commented Oct 30, 2023

Uh oh!

nodejs-github-bot commented Dec 4, 2023

Uh oh!

nodejs-github-bot commented Dec 18, 2023

Uh oh!

nodejs-github-bot commented Dec 19, 2023

Uh oh!

nodejs-github-bot commented Dec 19, 2023

Uh oh!

KuthorX commented Dec 19, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

KuthorXMay 30, 2020•
edited
Loading

KuthorX commented Dec 19, 2023•
edited
Loading