undici icon indicating copy to clipboard operation
undici copied to clipboard

Fetch Test Coverage

Open ronag opened this issue 3 years ago • 8 comments

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 avatar Aug 13 '21 06:08 ronag

@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?

teezzan avatar Aug 13 '21 15:08 teezzan

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

ronag avatar Aug 13 '21 15:08 ronag

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 https://github.com/nodejs/node/pull/41749#issuecomment-1024970687

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

trivikr avatar Jan 31 '22 15:01 trivikr

Could you propose some text?

mcollina avatar Jan 31 '22 21:01 mcollina

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:

  • https://github.com/nodejs/undici/pull/1191
  • https://github.com/nodejs/undici/pull/1192

trivikr avatar Jan 31 '22 23:01 trivikr

Done!

mcollina avatar Feb 01 '22 10:02 mcollina

Sent https://github.com/nodejs/undici/pull/1206 to complete the coverage of the File class in its current state.

RaisinTen avatar Feb 06 '22 11:02 RaisinTen

any updates on this becoming stable?

artrixdotdev avatar Sep 15 '22 18:09 artrixdotdev

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

ronag avatar Oct 26 '22 16:10 ronag

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.

KhafraDev avatar Oct 26 '22 16:10 KhafraDev

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

ronag avatar Oct 26 '22 17:10 ronag

[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?

KhafraDev avatar Oct 26 '22 17:10 KhafraDev