Skip to content
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 basic tests to wasm with modifying JS runtime with forked wasm_exec.js #5

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

hhhapz
Copy link
Member

@hhhapz hhhapz commented Mar 24, 2021

No description provided.

Copy link
Contributor

@chanbakjsd2 chanbakjsd2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about how testing is implemented currently.
It'll make it harder for people who use the lib to test their code, a Go program that does the setting up in a temporary directory and then executing it might be a better idea as that'll allow people to go install the binary at least.

```
GOOS=js GOARCH=wasm go get -u github.com/teamortix/golang-wasm/wasm
```
> To run tests, run [test.sh](./testing/test.sh)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
> To run tests, run [test.sh](./testing/test.sh)
> To run tests, run [test.sh](./testing/test.sh).

```bash
sh testing/test.sh . # Test all files in directory.

sh testing/wasm_test.go # Test only a single file.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are missing the test.sh argument.

> To run tests, run [test.sh](./testing/test.sh)

```bash
sh testing/test.sh . # Test all files in directory.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider just making testing/test.sh executable instead of forcing users to invoke it with sh explicitly.

}

func TestSetupCorrectly(t *testing.T) {
if funcWrapper.Type() != js.TypeFunction {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test order is not guaranteed so funcWrapper may not be set at this point in time. It'll be a better idea to just use the Expect API provided in Objects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants