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.

Sign up for GitHub

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

Jump to bottom

call_sync_on_main_thread tests are rubbish #8620

Closed
VirtualTim opened this issue May 16, 2019 · 0 comments · Fixed by #8621
Closed

call_sync_on_main_thread tests are rubbish #8620

VirtualTim opened this issue May 16, 2019 · 0 comments · Fixed by #8621

Comments

@VirtualTim
Copy link
Collaborator

Two issues:

  1. The js uses text = document.getElementById("resize").innerHTML, however that returns the inner html of <input type="checkbox" id="resize">, which is nothing ("").
  2. The c uses assert(!strstr(dst, "Resize canvas"));, where dst is erroneously the empty string.

I'll open a PR to actually make these tests work.
@VirtualTim VirtualTim mentioned this issue May 16, 2019
Merged
kripken pushed a commit that referenced this issue May 17, 2019
@VirtualTim @kripken
Fix the dodgy call_sync_on_main_thread tests (#8621)
736739d 
Previously these tests weren't actually checking the right thing. This should fix the logic to what I think the author was trying to do.

Fixes #8620.
VirtualTim added a commit to VirtualTim/emscripten that referenced this issue May 21, 2019
@VirtualTim
Fix the dodgy call_sync_on_main_thread tests (emscripten-core#8621)
15cda75 
Previously these tests weren't actually checking the right thing. This should fix the logic to what I think the author was trying to do.

Fixes emscripten-core#8620.
belraquib pushed a commit to belraquib/emscripten that referenced this issue Dec 23, 2020
@VirtualTim @belraquib
Fix the dodgy call_sync_on_main_thread tests (emscripten-core#8621)
3d8640e 
Previously these tests weren't actually checking the right thing. This should fix the logic to what I think the author was trying to do.

Fixes emscripten-core#8620.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix the dodgy call_sync_on_main_thread tests VirtualTim/emscripten
1 participant
@VirtualTim