Skip to content

Possibly fix flaky test seen to error in wasm2js3.test_stat_chmod_wasmfs. #24989

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

Merged
merged 3 commits into from
Aug 20, 2025
Merged

Possibly fix flaky test seen to error in wasm2js3.test_stat_chmod_wasmfs. #24989

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4aa2673
Fix flaky test seen to error in wasm2js3.test_stat_chmod_wasmfs.
juj Aug 20, 2025
17ff313
Type
juj Aug 20, 2025
b8abec4
Cast
juj Aug 20, 2025
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
6 changes: 3 additions & 3 deletions test/stat/test_chmod.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ void cleanup() {

void test() {
int err;
int lastctime;
int lastmtime;
time_t lastctime;
time_t lastmtime;
struct stat s;

//
Expand Down Expand Up @@ -129,7 +129,7 @@ void test() {

#ifndef WASMFS // TODO https://github.com/emscripten-core/emscripten/issues/15948
lstat("file-link", &s);
int link_mode = s.st_mode;
mode_t link_mode = s.st_mode;
assert((link_mode & 0777) != S_IRUSR);

//
Expand Down
2 changes: 1 addition & 1 deletion test/test_core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5686,7 +5686,7 @@ def test_stat_chmod(self):
self.skipTest('mode bits work differently on windows')
if nodefs and self.get_setting('WASMFS'):
self.skipTest('test requires symlink creation which currently missing from wasmfs+noderawfs')
self.do_runf('stat/test_chmod.c', 'success')
self.do_runf('stat/test_chmod.c', 'success', cflags=['-Werror=conversion'])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Makes me wonder if we should make an effort to enable this in whole test suite.

We currently enable -Werror by default:

emscripten/test/common.py

Line 1202 in 4cab039

self.cflags = ['-Wclosure', '-Werror', '-Wno-limited-postlink-optimizations']

But we don't enable -Wall (which looks like it doesn't actually include -Wconversion anyway )

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was expecting you might suggest that, and did give it a quick test before this PR, and was met with a million double -> float narrowing warnings with most tests failing miserably. It would be a nice idea, something to work on later.

I also searched the repo for other uses of .st_mtime and this was the only occurrence that assigned that to an int, other places have been converted already before.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, no worries, I think we are long way from -Wall or -Wextra by default in the test suite !


@also_with_wasmfs
def test_stat_mknod(self):
Expand Down