-
Notifications
You must be signed in to change notification settings - Fork 4.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
Add Wasm testing in Chromium, not just command-line v8
#40786
Changes from all commits
95706be
778a755
ce806e5
673fb32
ba3451d
0fe0f75
237fe1b
e666013
9f3d73c
e08cffe
10243dc
aec3f25
ffc5c9d
ef74cd6
f2914bf
06ea87b
571fa98
a5175b1
7cf7cb8
06540e1
e1be91b
2178fa5
ff417d0
0953fd9
fe94b9a
3854a80
af7cf7d
ea9d3f5
9dccea0
6d22ac6
f8f6713
ee3fc3b
63037d7
dea619b
1a52cad
ac7d4a6
becbfe7
d8100a6
4521654
0e9f7bd
1317668
90b7097
145722a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,9 +5,34 @@ | |
|
||
//glue code to deal with the differences between chrome, ch, d8, jsc and sm. | ||
var is_browser = typeof window != "undefined"; | ||
var consoleWebSocket; | ||
var print; | ||
|
||
if (is_browser) { | ||
// We expect to be run by tests/runtime/run.js which passes in the arguments using http parameters | ||
window.real_print = console.log; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I want this in asap but after it lands I think we should consider sending all this as json over the websocket and expand it on the xharness side, we should be able to clean up the console output considerably. pseudo code like
where method is { 'print', 'console.debug', ... } and after that it might make sense to add a custom log provider to preserve even more structure. |
||
print = function(_msg) { window.real_print(_msg); }; | ||
console.log = print; | ||
console.debug = print; | ||
console.error = print; | ||
console.trace = print; | ||
console.warn = print; | ||
console.info = print; | ||
|
||
const consoleUrl = `${window.location.origin}/console`.replace('http://', 'ws://'); | ||
|
||
consoleWebSocket = new WebSocket(consoleUrl); | ||
consoleWebSocket.onopen = function(event) { | ||
consoleWebSocket.send("browser: Console websocket connected."); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This might be a better place to force everything through print? I sometimes use runtime-tests.js outside of xharness There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, that makes sense. And I was thinking maybe the WebSocket thing should be optional? I found it useful to run the tests without the harness, directly in a browser. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it is worth a follow-up once we have everything working in CI. |
||
|
||
window.real_print = function(msg) { | ||
consoleWebSocket.send(msg); | ||
}; | ||
}; | ||
consoleWebSocket.onerror = function(event) { | ||
console.log(`websocket error: ${event}`); | ||
}; | ||
|
||
var url = new URL (decodeURI (window.location)); | ||
arguments = []; | ||
for (var v of url.searchParams) { | ||
|
@@ -18,9 +43,6 @@ if (is_browser) { | |
} | ||
} | ||
|
||
if (is_browser || typeof print === "undefined") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Um.. should this be:
.. for the non-browser case? |
||
print = console.log; | ||
|
||
// JavaScript core does not have a console defined | ||
if (typeof console === "undefined") { | ||
var Console = function () { | ||
|
@@ -151,6 +173,9 @@ while (args !== undefined && args.length > 0) { | |
} | ||
testArguments = args; | ||
|
||
// cheap way to let the testing infrastructure know we're running in a browser context (or not) | ||
setenv["IsBrowserDomSupported"] = is_browser.toString().toLowerCase(); | ||
|
||
function writeContentToFile(content, path) | ||
{ | ||
var stream = FS.open(path, 'w+'); | ||
|
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.
do we have a tracking issue for this?
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.
Should any of the SocketsHttpHandler tests be working?
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.
No, SocketsHttpHandler should throw PNSE on wasm-browser
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.
Ok, let's follow up on this.