Bug 163846

So I am confused as to how this test is passing on Mac, if WebAssembly is currently disabled everywhere. If this was a layout test, I would just skip the test. Do we have some mechanism for dealing with this in JS tests? If not, I guess it should be temporarily removed?

Oh, one more failure now: 30760/37506 ........... wasm.yaml/wasm/js-api/test_Module.js.default-wasm: Exception: ReferenceError: Can't find variable: WebAssembly 30760/37506 ........... wasm.yaml/wasm/js-api/test_Module.js.default-wasm: EmptyModule@/home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/Release/bin/jsc-stress-results/.tests/wasm.yaml/wasm/js-api/test_Module.js:7:35 30760/37506 ........... wasm.yaml/wasm/js-api/test_Module.js.default-wasm: module code@/home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/Release/bin/jsc-stress-results/.tests/wasm.yaml/wasm/js-api/test_Module.js:9:3 30760/37506 ........... wasm.yaml/wasm/js-api/test_Module.js.default-wasm: evaluate@[native code] 30760/37506 ........... wasm.yaml/wasm/js-api/test_Module.js.default-wasm: moduleEvaluation@[native code] 30760/37506 ........... wasm.yaml/wasm/js-api/test_Module.js.default-wasm: [native code] 30760/37506 ........... wasm.yaml/wasm/js-api/test_Module.js.default-wasm: promiseReactionJob@[native code] 30760/37506 ........... 30761/37506 .......... 30761/37506 .......... wasm.yaml/wasm/js-api/test_basic_api.js.default-wasm: ERROR: Unexpected exit code: 3 30761/37506 .......... 30762/37506 (failed 1) ......... 30762/37506 (failed 1) .......... wasm.yaml/wasm/js-api/test_Module.js.default-wasm: ERROR: Unexpected exit code: 3

I'm not an expert as how tests work, but I'm guessing the problem came from here: https://bugs.webkit.org/attachment.cgi?id=291661 When I added runWebAssembly: https://trac.webkit.org/browser/trunk/Tools/Scripts/run-jsc-stress-tests#L1157 Can you confirm that this is the case? Are the conditions at the top wrong for GTK? Sorry for asking all these questions, I don't understand how this all holds together.

(In reply to comment #3) > I'm not an expert as how tests work, but I'm guessing the problem came from > here: > https://bugs.webkit.org/attachment.cgi?id=291661 > > When I added runWebAssembly: > > https://trac.webkit.org/browser/trunk/Tools/Scripts/run-jsc-stress- > tests#L1157 > > Can you confirm that this is the case? Nope, I honestly don't know how to check JS test results that far back. :( I'm sure there's a way.... > Are the conditions at the top wrong for GTK? They look sane to me. > Sorry for asking all these questions, I don't understand how this all holds > together. Me either! I think we need to figure out how it is that WebAssembly works at all on the Mac port, then decide if it's ready to be turned on for other ports or not. If it's not ready yet, then we should find a way to skip the test conditional on whether or not it's enabled. Alternatively, we could modify the test to pass whenever the WebAssembly variable is not defined, and assume that we would notice if such a major feature accidentally disappeared.

> > Are the conditions at the top wrong for GTK? > > They look sane to me. Hmm, I don't think they are: $isFTLPlatform = !($architecture == "x86" || $architecture == "arm" || $hostOS == "windows" || $hostOS == "linux" && $architecture == "arm64") We'd need a way to see if the platform built with ENABLE(WEBASSEMBLY). I'm not sure how this is done. > Me either! I think we need to figure out how it is that WebAssembly works at > all on the Mac port, then decide if it's ready to be turned on for other > ports or not. If it's not ready yet, then we should find a way to skip the > test conditional on whether or not it's enabled. It's not ready yet, and we're only concentrating on Mac bits right now so some of the code will need Linux-isms to work well on Linux. I wouldn't enable it yet, unless someone is willing to do the Linux work. > Alternatively, we could modify the test to pass whenever the WebAssembly > variable is not defined, and assume that we would notice if such a major > feature accidentally disappeared. I'd rather not since the entire point is to test that WebAssembly works. It's unlikely that we'd mess that up, but still it's not released so nobody would notice.

(In reply to comment #5) > > > Are the conditions at the top wrong for GTK? > > > > They look sane to me. > > Hmm, I don't think they are: > > $isFTLPlatform = !($architecture == "x86" || $architecture == "arm" || > $hostOS == "windows" || $hostOS == "linux" && $architecture == "arm64") > > We'd need a way to see if the platform built with ENABLE(WEBASSEMBLY). I'm > not sure how this is done. > > > Me either! I think we need to figure out how it is that WebAssembly works at > > all on the Mac port, then decide if it's ready to be turned on for other > > ports or not. If it's not ready yet, then we should find a way to skip the > > test conditional on whether or not it's enabled. > > It's not ready yet, and we're only concentrating on Mac bits right now so > some of the code will need Linux-isms to work well on Linux. I wouldn't > enable it yet, unless someone is willing to do the Linux work. > > > > Alternatively, we could modify the test to pass whenever the WebAssembly > > variable is not defined, and assume that we would notice if such a major > > feature accidentally disappeared. > > I'd rather not since the entire point is to test that WebAssembly works. > It's unlikely that we'd mess that up, but still it's not released so nobody > would notice. It seems that ENABLE(WEBASSEMBLY) is ON in Mac ports by default, right? I think adding the following "darwin" check in runWebAssembly is preferable. if $hostOS == "darwin" WEBASSEMBLY flag can be enabled even if it is still under the development, and actually it seems that it is enabled in Mac ports. Why we can do that safely is there is runtime flag Options::useWebAssembly(). It prevents us from exposing the implementation under the heavy development. Basically, we can enable WEBASSEMBLY flag. The reason why we would like to disable this in Linux port is we have a part only focusing on Darwin right now. So, the reasonable choice is disabling the tests based on the $hostOS.

Created attachment 292863 [details] Patch

BTW, still the problem occurs in GTK port on Darwin. But I think we have a chance to enable it on Darwin.

Comment on attachment 292863 [details] Patch Looks good, though I think we should file a bug to remove this on Linux (and link to it from the comment). Thanks for looking into it, I wasn't sure what the best fix was :-)

Comment on attachment 292863 [details] Patch OK it looks sane, thanks. We don't have any GTK tests bots running Darwin so I don't really care toooo much if it's still broken on that platform, but indeed WebAssembly would not be available there either, so testing the port would make more sense than testing the OS, right?

(In reply to comment #10) > Comment on attachment 292863 [details] > Patch > > OK it looks sane, thanks. > > We don't have any GTK tests bots running Darwin so I don't really care toooo > much if it's still broken on that platform, but indeed WebAssembly would not > be available there either, so testing the port would make more sense than > testing the OS, right? (In reply to comment #10) > Comment on attachment 292863 [details] > Patch > > OK it looks sane, thanks. > > We don't have any GTK tests bots running Darwin so I don't really care toooo > much if it's still broken on that platform, but indeed WebAssembly would not > be available there either, so testing the port would make more sense than > testing the OS, right? Yeah, I think theoretically we can. But, in reality, I think it constantly causes some build/runtime errors until the implementation becomes ready to be ported. Anyway, this should be a transient workaround. As JF suggested, I'll open a bug and note this in FIXME.

Committed r207913: <http://trac.webkit.org/changeset/207913>