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

Fetch Test Coverage #951

Closed
ronag opened this issue Aug 13, 2021 · 13 comments
Closed

Fetch Test Coverage #951

ronag opened this issue Aug 13, 2021 · 13 comments
Labels
fetch good first issue Good for newcomers
Milestone

Comments

@ronag
Copy link
Member

ronag commented Aug 13, 2021

Current test coverage on fetch is too low for a stable release.

Please refer lib/fetch folder on CodeCov to check which components of fetch implementation lack unit tests. Comment on this issue on which component you would like to add test and post a PR!

Please refer CONTRIBUTING.md on how to run tests.

@ronag ronag added the fetch label Aug 13, 2021
@ronag ronag modified the milestones: 4.0, stable fetch Aug 13, 2021
@ronag ronag added the good first issue Good for newcomers label Aug 13, 2021
@teezzan
Copy link

teezzan commented Aug 13, 2021

@ronag, I would love to work on this. However, I am a little new to this. Can you give me more information about the issue and a little bit of direction?

@ronag
Copy link
Member Author

ronag commented Aug 13, 2021

Run ’npm run coverage’ see what parts of the code lacks test and make one in the test folder.

@trivikr
Copy link
Member

trivikr commented Jan 31, 2022

Run ’npm run coverage’ see what parts of the code lacks test and make one in the test folder.

I landed on this issue after reading about lack of test coverage in nodejs/node#41749 (comment)

Assuming this is a tracking issue to improve test coverage, would it be helpful to provide detailed instructions on how folks can help in the issue description? cc @ronag

Also, it would be helpful if a link to this issue is provided in README at https://github.com/nodejs/undici#undicifetchinput-init-promise

@mcollina
Copy link
Member

Could you propose some text?

@trivikr
Copy link
Member

trivikr commented Jan 31, 2022

Could you propose some text?

How about this for issue description?


Please refer lib/fetch folder on CodeCov to check which components of fetch implementation lack unit tests. Comment on this issue on which component you would like to add test and post a PR!

Please refer CONTRIBUTING.md on how to run tests.


I've posted the following PRs to improve discoverability/documentation:

mcollina pushed a commit that referenced this issue Feb 1, 2022
@mcollina
Copy link
Member

mcollina commented Feb 1, 2022

Done!

mcollina pushed a commit that referenced this issue Feb 1, 2022
@mcollina mcollina pinned this issue Feb 1, 2022
@mcollina mcollina changed the title Test Coverage Fetch Test Coverage Feb 1, 2022
RaisinTen added a commit to RaisinTen/undici that referenced this issue Feb 6, 2022
@RaisinTen
Copy link
Contributor

Sent #1206 to complete the coverage of the File class in its current state.

KhafraDev pushed a commit to KhafraDev/undici that referenced this issue Jun 23, 2022
KhafraDev pushed a commit to KhafraDev/undici that referenced this issue Jun 23, 2022
@Artrix9095
Copy link

any updates on this becoming stable?

@ronag
Copy link
Member Author

ronag commented Oct 26, 2022

@KhafraDev, any idea on the coverage stats from wpt?

@KhafraDev
Copy link
Member

KhafraDev commented Oct 26, 2022

I couldn't get any tools to work with it, but it's likely pretty high with the current test coverage.

edit: I actually got c8 to work, but I'm unsure if it takes coverage from all 4 wpt suites. I'll need to configure it some more and do some testing.

@ronag
Copy link
Member Author

ronag commented Oct 26, 2022

Would be great if we could show off your hard work with numbers! If nothing else to bring trust and confidence to our users.

@KhafraDev
Copy link
Member

KhafraDev commented Oct 26, 2022

[fetch]: Completed: 1307, failed: 137, success: 1170, expected failures: 137, unexpected failures: 0
[FileAPI]: Completed: 234, failed: 19, success: 215, expected failures: 19, unexpected failures: 0
[mimesniff]: Completed: 1898, failed: 1008, success: 890, expected failures: 1008, unexpected failures: 0
[xhr]: Completed: 36, failed: 0, success: 36, expected failures: 0, unexpected failures: 0
--------------|---------|----------|---------|---------|------------------------------------------------------------------------------------------------
File          | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
--------------|---------|----------|---------|---------|------------------------------------------------------------------------------------------------
All files     |   87.18 |    84.11 |   90.43 |   87.18 |                                                                                                
 body.js      |   91.87 |    92.56 |     100 |   91.87 | 70-71,242-247,279,288-289,352-354,360-385,409-413,417-420,425-426                              
 constants.js |   79.46 |    33.33 |       0 |   79.46 | 67-74,81-95                                                                                    
 dataURL.js   |   94.88 |    91.39 |      90 |   94.88 | 124-135,161-162,170-171,176-177,241-243,292-293,327-328,372-373,548-551                        
 file.js      |   68.95 |    92.68 |   47.36 |   68.95 | ...153,156-161,164-169,172-177,180-185,188-193,196-201,204-209,212-217,220-221,303-304,337-350 
 formdata.js  |   77.84 |    71.64 |    92.3 |   77.84 | ...-94,112-113,116-119,134-135,138-141,152-153,156-159,162-165,205-206,217-218,229-230,244-263 
 global.js    |   72.91 |    42.85 |     100 |   72.91 | 17-18,21-29,34-35                                                                              
 headers.js   |   93.09 |     89.9 |     100 |   93.09 | 59-64,199-203,266,268-269,366-371,383,403-404,415-416,427-428,443-444,464-469                  
 index.js     |   87.79 |    77.74 |   89.36 |   87.79 | ...0-1731,1745-1750,1798-1799,1838-1839,1844-1845,1878-1880,1945,1954-1955,1988-1994,2012-2013 
 request.js   |    93.4 |    90.62 |     100 |    93.4 | ...218,229-239,245-251,258-261,282-288,294-297,312-315,371-374,485-488,585-586,593-595,842-843 
 response.js  |   93.36 |    95.45 |      96 |   93.36 | 53-56,419-428,442-455,459-468,574-575                                                          
 symbols.js   |     100 |      100 |     100 |     100 |                                                                                                
 util.js      |   78.93 |    72.84 |    87.5 |   78.93 | ...462,465-504,517-518,525-526,530-531,592-597,611-612,715-718,837-840,850-851,854-856,868-869 
 webidl.js    |   82.32 |       80 |     100 |   82.32 | ...236,258-262,317-321,355-361,413-414,512-517,524-528,552-557,564-568,585-589,596-600,626-627 
--------------|---------|----------|---------|---------|------------------------------------------------------------------------------------------------

ran with
c8 --include=lib/fetch npm run test:wpt

current code cov for lib/fetch: ~92%
wpt code cov: 87.2%

if we can update codecov to combine all coverage files we'll likely see some increase?

metcoder95 pushed a commit to metcoder95/undici that referenced this issue Dec 26, 2022
metcoder95 pushed a commit to metcoder95/undici that referenced this issue Dec 26, 2022
@KhafraDev
Copy link
Member

I'm going to close this, we definitely have adequate test coverage.

crysmags pushed a commit to crysmags/undici that referenced this issue Feb 27, 2024
crysmags pushed a commit to crysmags/undici that referenced this issue Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fetch good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

7 participants