Skip to content

found/fixed some errors while testing agains WPT #109

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Jul 31, 2021

Conversation

jimmywarting
Copy link
Contributor

@jimmywarting jimmywarting commented Jul 30, 2021

Manage to find a way to run test agains WPT with experimental https loader in nodejs 😉

Test like this one currently failed but it got fixed now in this PR:

await new Blob([
	new Uint8Array([0, 255, 0])).buffer,
	new Blob(['abcd']),
	'efgh',
	'ijklmnopqrstuvwxyz'
]).slice(1, 12).text() === "\uFFFD\u0000abcdefghi"

Filename wasn't converted to a string sometimes
and new Blob('abc') should never have worked...

closes #100

@codecov
Copy link

codecov bot commented Jul 30, 2021

Codecov Report

Merging #109 (04c0019) into master (c2903ca) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #109   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            4         4           
  Lines          412       424   +12     
  Branches        60        69    +9     
=========================================
+ Hits           412       424   +12     
Impacted Files Coverage Δ
file.js 100.00% <100.00%> (ø)
index.js 100.00% <100.00%> (ø)

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 c2903ca...04c0019. Read the comment docs.

@octet-stream
Copy link
Contributor

and new Blob('abc') should never have worked...

Ah, right, my mistake. I just thought if new String('abc') passes than "abc" should work too, because both of them has @@iterator method (when you iterate over a regular string, it will be just treat as a String object by JS, afaik). Should've just stick with the implementation from that WPT test :D

@octet-stream
Copy link
Contributor

octet-stream commented Jul 31, 2021

I was thinking if I will be able to run tests agains WPT for formdata-node v4.🤔
If there's any of them for form-data ofc.

@jimmywarting
Copy link
Contributor Author

jimmywarting commented Jul 31, 2021

I have helped out Deno implement a async blob source (while doing so i found some quirks that was done incorrectly - they run tests agains WPT...)
It resulted in breaking Deno's formdata, createObjectURL & fetch cuz they read the files content from something like a private property containing the hole underlying buffer held in memory from the blob (like how we used to do before #40 was fixed). b/c of that i also updated their formdata encoder to use my formDataToBlob impl

@jimmywarting jimmywarting merged commit b86b140 into master Jul 31, 2021
@jimmywarting jimmywarting deleted the feature/test-agains-wpt branch July 31, 2021 09:24
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Run test against WPT
3 participants