-
Notifications
You must be signed in to change notification settings - Fork 10
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
Prepare for creating wasm builds #32
base: master
Are you sure you want to change the base?
Conversation
I checked the PR again and found nothing relevant, I just changed parts of the documentation and added comments to the Go file. I also made changes to the Travis config, and deactivated the checks for the GopherJS version in Travis (as those need Go v1.10 and the wasm build needs Go v1.12). However, it is still necessary to make changes to the Go linters config. |
With the lastest commit the wasm test does not only includes the cipher test suite (which was translated from Go to TypeScript), but also compiles the test files found in Currently the Go wasm builds don’t return the exit code to the calling js code without making modifications, but as a warning is shown in the console, the tests code detect that warning for failing. |
Merge conflicts |
In the last 2 commits, due to #34, the following changes were made:
|
@gz-c The last 2 commits solve the remaining problems with the linter and the tests. However, for some reason Travis made this build: https://travis-ci.com/skycoin/skycoin-lite/builds/119038575 but did not report the result here. |
Nevermind, Travis just taked a lot of time before reporting the result on Github |
@@ -31,5 +31,4 @@ required = ["github.com/gopherjs/gopherjs"] | |||
name = "github.com/skycoin/skycoin" | |||
|
|||
[prune] | |||
go-tests = true |
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.
I believe this defaults to true, you may want to be explicit go-tests = false
and leave a comment why (for the wasm tests)
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.
Adding go-tests = false
makes dep ensure
say root prune options must be omitted instead of being set to false
cd js && npm run test-extensive | ||
|
||
test-suite-ts-wasm: ## Run the ts version of the cipher test suite for wasm and additional tests | ||
cd vendor/github.com/skycoin/skycoin/src/cipher/secp256k1-go && GOOS=js GOARCH=wasm go test -c -o test.wasm |
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.
Can we run these tests for cipher/
as well?
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.
Yes, no problem. Done in the lastest commit
The last commit includes the following changes:
|
Changes:
Now it is possible to compile a wasm version of the library. The code was created for Go v1.12.x. it does not work with Go v1.11.x and will probably not work with Go v1.13.x, as the wasm compiler is still getting breaking changes.
A version of the cipher test suite for the wasm version was added.
The file
wasm_exec.js
is https://github.com/golang/go/blob/master/misc/wasm/wasm_exec.js for Go v1.12.The possibility of using GopherJS is still active, just in case. However, the wasm version is much faster, so the GopherJS version should be removed if we do not find problems.
I will check this PR again latter, just in case.