Skip to content

Cleanup the emscripten tests relating to the event loop. NFC #23760

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Cleanup the emscripten tests relating to the event loop. NFC #23760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions test/emscripten_performance_now.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@ void test(void *userData) {
double now3 = emscripten_date_now();
assert(now3 >= dateNow + 100);

#ifdef REPORT_RESULT
REPORT_RESULT(0);
#endif
exit(0);
}

int main() {
Expand All @@ -30,4 +28,5 @@ int main() {
#else
emscripten_set_timeout(test, 200, 0);
#endif
return 99;
}
3 changes: 2 additions & 1 deletion test/emscripten_set_interval.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
int funcExecuted = 0;

void testDone(void *userData) {
printf("testDone\n");
assert((long)userData == 2);
assert(funcExecuted == 10);
exit(0);
Expand All @@ -15,6 +16,7 @@ void testDone(void *userData) {
long intervalId = 0;

void tick(void *userData) {
printf("tick: %d\n", funcExecuted);
assert((long)userData == 1);
++funcExecuted;
if (funcExecuted == 10) {
Expand All @@ -28,6 +30,5 @@ void tick(void *userData) {

int main() {
intervalId = emscripten_set_interval(tick, 100, (void*)1);
emscripten_exit_with_live_runtime();
return 99;
}
16 changes: 10 additions & 6 deletions test/emscripten_set_timeout.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,34 +1,39 @@
#include <emscripten/emscripten.h>
#include <emscripten/html5.h>
#include <emscripten/em_asm.h>
#include <assert.h>
#include <stdlib.h>
#include <stdio.h>

int func1Executed = 0;
int func2Executed = 0;

void func1(void *userData);

void func2(void *userData) {
printf("func2: %d\n", func2Executed);
assert((long)userData == 2);
++func2Executed;

if (func2Executed == 1)
{
if (func2Executed == 1) {
// Test canceling a setTimeout: register a callback but then cancel it immediately
long id = emscripten_set_timeout(func1, 10, (void*)2);
emscripten_clear_timeout(id);

// Without this, the test will not exit correctly
// https://github.com/emscripten-core/emscripten/issues/23763
emscripten_runtime_keepalive_pop();

emscripten_set_timeout(func2, 100, (void*)2);
}
if (func2Executed == 2)
{

if (func2Executed == 2) {
assert(func1Executed == 1);
exit(0);
}
}

void func1(void *userData) {
printf("func1\n");
assert((long)userData == 1);
++func1Executed;

Expand All @@ -39,6 +44,5 @@ void func1(void *userData) {

int main() {
emscripten_set_timeout(func1, 100, (void*)1);
emscripten_exit_with_live_runtime();
return 99;
}
10 changes: 6 additions & 4 deletions test/emscripten_set_timeout_loop.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,32 +1,34 @@
#include <emscripten/emscripten.h>
#include <emscripten/html5.h>
#include <emscripten/em_asm.h>
#include <assert.h>
#include <stdlib.h>
#include <stdio.h>

double previousSetTimeouTime = 0;
int funcExecuted = 0;

void testDone(void *userData) {
printf("testDone\n");
assert((long)userData == 2);
assert(funcExecuted == 10);
emscripten_runtime_keepalive_pop();
exit(0);
}

bool tick(double time, void *userData) {
printf("tick: %d\n", funcExecuted);
assert(time >= previousSetTimeouTime);
previousSetTimeouTime = time;
assert((long)userData == 1);
++funcExecuted;
if (funcExecuted == 10)
{
if (funcExecuted == 10) {
emscripten_set_timeout(testDone, 300, (void*)2);
}
return funcExecuted < 10;
}

int main() {
emscripten_set_timeout_loop(tick, 100, (void*)1);
emscripten_exit_with_live_runtime();
emscripten_runtime_keepalive_push();
return 99;
}
95 changes: 46 additions & 49 deletions test/test_browser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,22 @@ def decorated(self, threads, *args, **kwargs):
f(self, *args, **kwargs)

parameterize(decorated, {'': (False,),
'pthreads': (True,)})
'pthread': (True,)})

return decorated


def also_with_proxy_to_pthread(f):
assert callable(f)

@wraps(f)
def decorated(self, threads, *args, **kwargs):
if threads:
self.emcc_args += ['-pthread', '-sPROXY_TO_PTHREAD']
f(self, *args, **kwargs)

parameterize(decorated, {'': (False,),
'pthread': (True,)})

return decorated

Expand Down Expand Up @@ -496,14 +511,11 @@ def make_main_two_files(path1, path2, nonexistingpath):
self.btest_exit('main.c', args=['--pre-js', 'pre.js', '--use-preload-plugins'])

# Tests that user .html shell files can manually download .data files created with --preload-file cmdline.
@parameterized({
'': ([],),
'pthreads': (['-pthread', '-sPROXY_TO_PTHREAD', '-sEXIT_RUNTIME'],),
})
def test_preload_file_with_manual_data_download(self, args):
@also_with_proxy_to_pthread
def test_preload_file_with_manual_data_download(self):
create_file('file.txt', 'Hello!')

self.compile_btest('browser/test_manual_download_data.c', ['-sEXIT_RUNTIME', '-o', 'out.js', '--preload-file', 'file.txt@/file.txt'] + args)
self.compile_btest('browser/test_manual_download_data.c', ['-sEXIT_RUNTIME', '-o', 'out.js', '--preload-file', 'file.txt@/file.txt'])
shutil.copy(test_file('browser/test_manual_download_data.html'), '.')

# Move .data file out of server root to ensure that getPreloadedPackage is actually used
Expand Down Expand Up @@ -1603,12 +1615,9 @@ def test_glfw_time(self):
def test_egl(self, args):
self.btest_exit('test_egl.c', args=['-O2', '-lEGL', '-lGL', '-sGL_ENABLE_GET_PROC_ADDRESS'] + args)

@parameterized({
'': ([],),
'proxy_to_pthread': (['-pthread', '-sPROXY_TO_PTHREAD'],),
})
def test_egl_width_height(self, args):
self.btest_exit('test_egl_width_height.c', args=['-O2', '-lEGL', '-lGL'] + args)
@also_with_proxy_to_pthread
def test_egl_width_height(self):
self.btest_exit('test_egl_width_height.c', args=['-O2', '-lEGL', '-lGL'])

@requires_graphics_hardware
def test_egl_createcontext_error(self):
Expand Down Expand Up @@ -1901,27 +1910,17 @@ def test_emscripten_fs_api2(self):
self.btest_exit('emscripten_fs_api_browser2.c', args=['-sASSERTIONS=0'])
self.btest_exit('emscripten_fs_api_browser2.c', args=['-sASSERTIONS=1'])

@parameterized({
'': ([],),
'pthreads': (['-pthread', '-sPROXY_TO_PTHREAD', '-sEXIT_RUNTIME'],),
})
def test_emscripten_main_loop(self, args):
self.btest_exit('test_emscripten_main_loop.c', args=args)
@also_with_proxy_to_pthread
def test_emscripten_main_loop(self):
self.btest_exit('test_emscripten_main_loop.c')

@parameterized({
'': ([],),
# test pthreads + AUTO_JS_LIBRARIES mode as well
'pthreads': (['-pthread', '-sPROXY_TO_PTHREAD', '-sAUTO_JS_LIBRARIES=0'],),
})
def test_emscripten_main_loop_settimeout(self, args):
self.btest_exit('test_emscripten_main_loop_settimeout.c', args=args)
@also_with_proxy_to_pthread
def test_emscripten_main_loop_settimeout(self):
self.btest_exit('test_emscripten_main_loop_settimeout.c', args=['-sAUTO_JS_LIBRARIES=0'])

@parameterized({
'': ([],),
'pthreads': (['-pthread', '-sPROXY_TO_PTHREAD'],),
})
def test_emscripten_main_loop_and_blocker(self, args):
self.btest_exit('test_emscripten_main_loop_and_blocker.c', args=args)
@also_with_proxy_to_pthread
def test_emscripten_main_loop_and_blocker(self):
self.btest_exit('test_emscripten_main_loop_and_blocker.c')

def test_emscripten_main_loop_and_blocker_exit(self):
# Same as above but tests that EXIT_RUNTIME works with emscripten_main_loop. The
Expand Down Expand Up @@ -1977,13 +1976,10 @@ def test_sdl_glshader2(self):
def test_gl_glteximage(self):
self.btest('gl_teximage.c', '1', args=['-lGL', '-lSDL'])

@parameterized({
'': ([],),
'pthreads': (['-pthread', '-sPROXY_TO_PTHREAD', '-sOFFSCREEN_FRAMEBUFFER'],),
})
@requires_graphics_hardware
def test_gl_textures(self, args):
self.btest_exit('gl_textures.c', args=['-lGL', '-g', '-sSTACK_SIZE=1MB'] + args)
@also_with_proxy_to_pthread
def test_gl_textures(self):
self.btest_exit('gl_textures.c', args=['-lGL', '-g', '-sSTACK_SIZE=1MB', '-sOFFSCREEN_FRAMEBUFFER'])
Copy link
Member

Choose a reason for hiding this comment

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

This did not always test with OFFSCREEN_FRAMEBUFFER before. Do we not error on that being used without pthreads?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, I think think its harmlessly ignored.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe we should error ? But we don't today.


@requires_graphics_hardware
def test_gl_ps(self):
Expand Down Expand Up @@ -2618,10 +2614,10 @@ def test_html5_core(self, opts):
self.emcc_args.append('--pre-js=pre.js')
self.btest_exit('test_html5_core.c', args=opts)

@also_with_proxy_to_pthread
@parameterized({
'': ([],),
'closure': (['-O2', '-g1', '--closure=1'],),
'pthread': (['-pthread', '-sPROXY_TO_PTHREAD'],),
})
def test_html5_gamepad(self, args):
self.btest_exit('test_gamepad.c', args=args)
Expand Down Expand Up @@ -4932,24 +4928,28 @@ def test_emscripten_request_animation_frame_loop(self):
def test_request_animation_frame(self):
self.btest_exit('test_request_animation_frame.c')

@also_with_proxy_to_pthread
def test_emscripten_set_timeout(self):
self.btest_exit('emscripten_set_timeout.c', args=['-pthread', '-sPROXY_TO_PTHREAD'])
self.btest_exit('emscripten_set_timeout.c')

@also_with_proxy_to_pthread
def test_emscripten_set_timeout_loop(self):
self.btest_exit('emscripten_set_timeout_loop.c', args=['-pthread', '-sPROXY_TO_PTHREAD'])
self.btest_exit('emscripten_set_timeout_loop.c')

def test_emscripten_set_immediate(self):
self.btest_exit('emscripten_set_immediate.c')

def test_emscripten_set_immediate_loop(self):
self.btest_exit('emscripten_set_immediate_loop.c')

@also_with_proxy_to_pthread
def test_emscripten_set_interval(self):
self.btest_exit('emscripten_set_interval.c', args=['-pthread', '-sPROXY_TO_PTHREAD'])
self.btest_exit('emscripten_set_interval.c')

# Test emscripten_performance_now() and emscripten_date_now()
@also_with_proxy_to_pthread
def test_emscripten_performance_now(self):
self.btest('emscripten_performance_now.c', '0', args=['-pthread', '-sPROXY_TO_PTHREAD'])
self.btest_exit('emscripten_performance_now.c')

def test_embind_with_pthreads(self):
self.btest_exit('embind/test_pthreads.cpp', args=['-lembind', '-pthread', '-sPTHREAD_POOL_SIZE=2'])
Expand Down Expand Up @@ -5021,12 +5021,9 @@ def test_minimal_runtime_loader_shell(self, args):
def test_minimal_runtime_hello_world(self, args):
self.btest_exit('small_hello_world.c', args=args + ['-sMINIMAL_RUNTIME'])

@parameterized({
'': ([],),
'pthread': (['-sPROXY_TO_PTHREAD', '-pthread'],)
})
def test_offset_converter(self, args):
self.btest_exit('test_offset_converter.c', args=['-sUSE_OFFSET_CONVERTER', '-gsource-map'] + args)
@also_with_proxy_to_pthread
def test_offset_converter(self):
self.btest_exit('test_offset_converter.c', args=['-sUSE_OFFSET_CONVERTER', '-gsource-map'])

# Tests emscripten_unwind_to_js_event_loop() behavior
def test_emscripten_unwind_to_js_event_loop(self):
Expand Down
9 changes: 3 additions & 6 deletions test/test_other.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13883,9 +13883,8 @@ def test_pthread_trap(self):
def test_pthread_kill(self):
self.do_run_in_out_file_test('pthread/test_pthread_kill.c')

@node_pthreads
def test_emscripten_set_interval(self):
self.do_runf('emscripten_set_interval.c', args=['-pthread', '-sPROXY_TO_PTHREAD'])
self.do_runf('emscripten_set_interval.c')
Copy link
Member

Choose a reason for hiding this comment

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

Previously this was tested with pthreads, but now it isn't, if I read this diff correctly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, see the PR description. There is a typo here. args should be emcc_args so this was never working here. args in this context refers to arguments for the program itself.

Copy link
Member

Choose a reason for hiding this comment

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

I see. Should we fix the typo and test it with pthreads, or not?

Copy link
Member

Choose a reason for hiding this comment

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

I can't think of a reason to use pthreads on this myself. lgtm either way.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We test with and without threads in the browser already.


# Test emscripten_console_log(), emscripten_console_warn() and emscripten_console_error()
def test_emscripten_console_log(self):
Expand All @@ -13895,13 +13894,11 @@ def test_emscripten_console_log(self):
def test_emscripten_unwind_to_js_event_loop(self):
self.do_runf('test_emscripten_unwind_to_js_event_loop.c')

@node_pthreads
def test_emscripten_set_timeout(self):
self.do_runf('emscripten_set_timeout.c', args=['-pthread', '-sPROXY_TO_PTHREAD'])
self.do_runf('emscripten_set_timeout.c')

@node_pthreads
def test_emscripten_set_timeout_loop(self):
self.do_runf('emscripten_set_timeout_loop.c', args=['-pthread', '-sPROXY_TO_PTHREAD'])
self.do_runf('emscripten_set_timeout_loop.c')

# Verify that we are able to successfully compile a script when the Windows 7
# and Python workaround env. vars are enabled.
Expand Down