Skip to content

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Dec 16, 2024

  • @with_all_fs
  • @also_with_nodefs
  • @also_with_nodefs_both

@sbc100 sbc100 changed the title [test] Add new test decorators for running tests under different FS b… [test] Add new decorators for running tests under different FS backends Dec 16, 2024
@sbc100 sbc100 requested review from hoodmane and kripken December 16, 2024 18:31
@sbc100 sbc100 force-pushed the with_all_fs branch 3 times, most recently from f59bf86 to 93b7e51 Compare December 16, 2024 23:41
Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

lgtm % question

Btw, separately, this adds more work in test_core that could all possibly be in other - not sure we need to run it in all opt modes.

@sbc100
Copy link
Collaborator Author

sbc100 commented Dec 17, 2024

lgtm % question

Btw, separately, this adds more work in test_core that could all possibly be in other - not sure we need to run it in all opt modes.

I think in almost all of these cases I'm not adding more test variants here, just converting existing variants to use the decorators instead. There may be one or two where I increased it. I agree we might want to move all of these highly-parameterized FS tests to other. Perhaps as a followup.

@sbc100 sbc100 force-pushed the with_all_fs branch 2 times, most recently from 85a7325 to 1fec631 Compare December 17, 2024 00:46
@sbc100 sbc100 enabled auto-merge (squash) December 17, 2024 00:46
…ackends

- @with_all_fs
- @also_with_nodefs
- @also_with_nodefs_both
@sbc100 sbc100 disabled auto-merge December 17, 2024 19:18
@sbc100 sbc100 merged commit c17479b into emscripten-core:main Dec 17, 2024
22 of 28 checks passed
@sbc100 sbc100 deleted the with_all_fs branch December 17, 2024 19:19
hedwigz pushed a commit to hedwigz/emscripten that referenced this pull request Dec 18, 2024
…ds (emscripten-core#23177)

- @with_all_fs
- @also_with_nodefs
- @also_with_nodefs_both
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.

3 participants