Skip to content

Allow setting of CLOSURE_ARGS in ports #24192

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

Merged
merged 7 commits into from
Apr 28, 2025

Conversation

ypujante
Copy link
Contributor

This is the implementation that was discussed in #24109

I can update the Changelog in a later commit if the tests pass in CI and this PR is approved.

  • I manually/locally ran other.test_closure_externs to make sure that the changes were not affecting existing code
  • I added a test other.test_closure_args_from_port that piggy back on test_closure_externs to pass the same parameter in the port instead and make sure that these changes actually do what they are supposed to do in the first place.

Hopefully I didn't forget anything. The changes are pretty straightforward in the grand scheme of things.

@ypujante
Copy link
Contributor Author

This is the only test that fails. It doesn't seem to be related to my changes but please let me know if it is.

======================================================================
FAIL [2.055s]: test_sdl_mouse (test_browser.browser_2gb)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/root/project/test/common.py", line 2353, in run_browser
    self.assertContained(expected, output)
  File "/root/project/test/common.py", line 1652, in assertContained
    self.fail("Expected to find '%s' in '%s', diff:\n\n%s\n%s" % (
  File "/usr/lib/python3.8/unittest/case.py", line 753, in fail
    raise self.failureException(msg)
AssertionError: Expected to find '/report_result?exit:0
' in '//report_result?abort:Assertion failed: abs(m->x-10) <= 1 && abs(m->y-20) <= 1 && abs(m->xrel-10) <= 1 && abs(m->yrel-20) <= 1, at: /root/project/test/browser/test_sdl_mouse.c,32,one
', diff:

--- expected
+++ actual
@@ -1 +1 @@
-/report_result?exit:0
+//report_result?abort:Assertion failed: abs(m->x-10) <= 1 && abs(m->y-20) <= 1 && abs(m->xrel-10) <= 1 && abs(m->yrel-20) <= 1, at: /root/project/test/browser/test_sdl_mouse.c,32,one



During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/root/project/test/test_browser.py", line 1070, in test_sdl_mouse
    self.btest_exit('test_sdl_mouse.c', emcc_args=['-O2', '--minify=0', '--pre-js', test_file('browser/fake_events.js'), '-lSDL', '-lGL'])
  File "/root/project/test/common.py", line 2404, in btest_exit
    return self.btest(filename, *args, **kwargs)
  File "/root/project/test/common.py", line 2433, in btest
    self.run_browser(outfile + url_suffix, expected=['/report_result?' + e for e in expected], timeout=timeout, extra_tries=extra_tries)
  File "/root/project/test/common.py", line 2358, in run_browser
    return self.run_browser(html_file, expected, message, timeout, extra_tries - 1)
  File "/root/project/test/common.py", line 2360, in run_browser
    raise e
  File "/root/project/test/common.py", line 2353, in run_browser
    self.assertContained(expected, output)
  File "/root/project/test/common.py", line 1652, in assertContained
    self.fail("Expected to find '%s' in '%s', diff:\n\n%s\n%s" % (
AssertionError: Expected to find '/report_result?exit:0
' in '//report_result?abort:Assertion failed: abs(m->x-10) <= 1 && abs(m->y-20) <= 1 && abs(m->xrel-10) <= 1 && abs(m->yrel-20) <= 1, at: /root/project/test/browser/test_sdl_mouse.c,32,one
', diff:

--- expected
+++ actual
@@ -1 +1 @@
-/report_result?exit:0
+//report_result?abort:Assertion failed: abs(m->x-10) <= 1 && abs(m->y-20) <= 1 && abs(m->xrel-10) <= 1 && abs(m->yrel-20) <= 1, at: /root/project/test/browser/test_sdl_mouse.c,32,one

@sbc100 sbc100 changed the title allow using settings.CLOSURE_ARGS in ports Allow setting of CLOSURE_ARGS in ports Apr 27, 2025
@sbc100
Copy link
Collaborator

sbc100 commented Apr 27, 2025

The test_sdl_mouse is a know flake that has been happening for a few days now.

@sbc100 sbc100 merged commit 36423eb into emscripten-core:main Apr 28, 2025
24 of 28 checks passed
copybara-service bot pushed a commit to google/dawn that referenced this pull request Apr 28, 2025
Support added in Emscripten in the next release:
emscripten-core/emscripten#24192

Tested with Emscripten 4.0.3 and emsdk's `tot` release.

No-Try: true
Bug: 371024051
Change-Id: I63eb5d6c8cdf43389cf9c09876acddd7b3f0a2a4
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/239474
Auto-Submit: Kai Ninomiya <[email protected]>
Commit-Queue: Loko Kung <[email protected]>
Reviewed-by: Loko Kung <[email protected]>
copybara-service bot pushed a commit to google/dawn that referenced this pull request May 1, 2025
This version supports port files setting --closure-args, so we want it
to be able to simplify build steps.
emscripten-core/emscripten#24192

Bug: 414330682
Change-Id: I7123613c064bfb763693fcb741c41aee4818506a
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/239494
Reviewed-by: Loko Kung <[email protected]>
Commit-Queue: Loko Kung <[email protected]>
@ypujante ypujante deleted the issue-24109 branch May 23, 2025 16:50
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