agnivade / wasmbrowsertest

Run WASM tests inside your browser
MIT License
191 stars 22 forks source link

Failed to run while using Go Toolchain #61

Closed Zxilly closed 1 month ago

Zxilly commented 5 months ago

When running the test with the go toolchain enabled, the GOROOT is set to the path of the go toolchain package.

However, it doesn't contain the misc folder, which leads to an error like

FAIL github.com/Zxilly/go-size-analyzer/internal/disasm 0.071s
open C:\Users\zxilly\go\pkg\mod\golang.org\toolchain@v0.0.1-go1.22.4.windows-amd64/misc/wasm/wasm_exec.js: The system cannot find the path specified.

Since this package was the official suggested test runner, I think this situation should be considered.

https://github.com/golang/go/wiki/WebAssembly#running-tests-in-the-browser

agnivade commented 5 months ago

How are you installing the toolchain? Are you using go install golang.org/dl/go1.22.4@latest? I am not sure how the GOROOT is pointing to C:\Users\zxilly\go\pkg\mod\golang.org\toolchain, because on my machine (using Linux), it's at $HOME/sdk.

I just ran a fresh test using 1.22.4

GOARCH=wasm GOOS=js go1.22.4 test -v
=== RUN   TestSanity
--- PASS: TestSanity (0.00s)
=== RUN   TestUnicode
--- PASS: TestUnicode (0.00s)
PASS
ok      github.com/agnivade/levenshtein 1.167s
14:38:52-~/play/agnivade/levenshtein] go1.22.4 env GOROOT
/home/agniva/sdk/go1.22.4
14:40:35-~/play/agnivade/levenshtein] ls /home/agniva/sdk/go1.22.4/misc/wasm/wasm_exec.js
/home/agniva/sdk/go1.22.4/misc/wasm/wasm_exec.js

Could you run an env GOROOT on your binary and paste the output?

Zxilly commented 5 months ago
(gsv-hyTHdi4q-py3.12) E:\Project\CS_Project\gsv\scripts git:[master]
go env GOROOT
C:/Users/zxilly/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.4.windows-amd64

to reproduce, create a dir with the go.mod

module test
go 1.22
toolchain go1.22.4

then run go mod download.

After these, you will see the GOROOT points to the go toolchain dir.

Zxilly commented 5 months ago

Go Toolchain here means the new feature introduced in Go1.21. Please see https://go.dev/doc/toolchain.

Zxilly commented 5 months ago

Google's developers decided not to include the misc folder in the toolchain package, which can be found discussed at https://github.com/golang/go/issues/68024. In this case, we need to figure out how to get the files ourselves. I created my own node.js based wrapper with the logic https://github.com/Zxilly/go_js_wasm_exec/blob/ad3f10d1c513fc1f9a192b7d816d44c2bce4eccd/ensure.go#L61-L101 can be used as a reference.

agnivade commented 5 months ago

Ahh I see. Well, that's an interesting one. So in that case, I think the reasonable workaround would be to set the env var for GOROOT to the binary toolchain distribution that you have locally.

Zxilly commented 5 months ago

But this may break other things use the same logic but read the exist file. They will read the wrong edition. Also, if wasm_exec.js got a update in new go version, this will make the user still works with the old one. I think set a workaround for it is reasonable.

agnivade commented 5 months ago

Yes, the main binary distribution should atleast be the same minor version as the toolchain distribution. If you are using a newer minor version from the Go toolchain, then of course there will be issues.

But more importantly, this is only while running tests so you have more flexibility in how you run them. You can either choose to avoid using the downloaded toolchain, but instead use the binary distribution. Or you can use the toolchain version, but just vary in patch version from the binary toolchain.

Zxilly commented 5 months ago

I still think adding additional logic is necessary, toolchain can be implicitly enabled by go.mod and environment variables, and troubleshooting the problem can be very difficult for people who don't know this mechanism. But this is your project and I respect your opinion.

Zxilly commented 5 months ago

And does GOROOT affect the go compiler's choice of standard libraries? Some problem that was fixed in the minor version might be reappeared by this operation.

agnivade commented 5 months ago

Thanks. On further thoughts, I think the best way is to have the directory included in the toolchain build. I have pinged in the issue for it to be reconsidered. Let's see.

Zxilly commented 5 months ago

Maybe you can pin this issue to let the users know what happened. Even if the proposal is accepted, we will need to wait for the next release to include these files. Frankly, I'm not very confident in the efficiency of the Go team.

agnivade commented 1 month ago

@Zxilly - I believe this issue can be closed now? Let me know if there is anything else needed.

Zxilly commented 1 month ago

https://github.com/agnivade/wasmbrowsertest/blob/06679196c7e76f227e71456cdc16fccd6cc33601/handler.go#L58

needs a fallback logic for new file location