-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
test: fix typos #55341
base: main
Are you sure you want to change the base?
test: fix typos #55341
Conversation
Review requested:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -51,7 +51,7 @@ | |||
|
|||
xhr.send(); | |||
|
|||
}, 'Check whether the browser response 500 in XHR if the selected file which File/Blob URL refered is not found'); | |||
}, 'Check whether the browser response 500 in XHR if the selected file which File/Blob URL referred is not found'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't work. Everything inside test/fixtures/wpt
is pulled from web-platform-tests/wpt, and we frequently pull in upstream changes using git node wpt
(see docs). Any changes to these files should be committed on the upstream repository instead, and then pulled back into Node afterwards.
I see that #55063 also touched these files. I highly recommend we revert those changes, since they'll be lost whenever we run git node wpt
again. (@panva already spotted this on the previous PR, but it seems like their remark was lost in the review process?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. All wpt
fixes reverted including the ones in my other merged PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW, I did consider giving wpt
the full treatment upstream but it's absolutely riddled with typos so I shelved it for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC test/fixtures/postject-copy
is an upstream project, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like I accidentally traversed into a node_modules
folder too. 😬
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upstreamed and reverted in this PR.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #55341 +/- ##
==========================================
- Coverage 88.41% 88.41% -0.01%
==========================================
Files 653 653
Lines 187476 187476
Branches 36083 36084 +1
==========================================
- Hits 165763 165759 -4
- Misses 14946 14955 +9
+ Partials 6767 6762 -5 |
b7b3e45
to
7654f17
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
7654f17
to
22fb682
Compare
22fb682
to
1cd0491
Compare
This is a continuation of #55063 covering all the non-comment fixes.
FWIW, the top typos are: