Skip to content

Check for expected error codes when running generated JS code#11578

Merged
sbc100 merged 6 commits intomasterfrom
assert_on_expected_failures
Jul 9, 2020
Merged

Check for expected error codes when running generated JS code#11578
sbc100 merged 6 commits intomasterfrom
assert_on_expected_failures

Conversation

@sbc100
Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 commented Jul 8, 2020

Previously we just disabled the check for returning 0. Now we
actually check that the program fails.

@sbc100 sbc100 requested a review from kripken July 8, 2020 02:30
@sbc100 sbc100 force-pushed the assert_on_expected_failures branch from dbf2096 to a04eb12 Compare July 8, 2020 03:30
@sbc100 sbc100 force-pushed the assert_on_expected_failures branch from aa0d41b to 0edecc4 Compare July 8, 2020 20:07
@sbc100 sbc100 changed the title Check for expected errors codes when running generated JS code Check for expected error codes when running generated JS code Jul 8, 2020
@sbc100 sbc100 force-pushed the assert_on_expected_failures branch 3 times, most recently from 986312b to 6d6bc42 Compare July 8, 2020 22:39
@sbc100
Copy link
Copy Markdown
Collaborator Author

sbc100 commented Jul 8, 2020

This should be good to go now..

sbc100 added 5 commits July 8, 2020 16:06
Previously we just disabled the check for returning 0.  Now we
actually check that the program fails.

assert_returncode=None now means assert that the subprocess fails with
a non-zero error code.  This is like specifying a positive integer
value but checkes for any positive value.
@sbc100 sbc100 force-pushed the assert_on_expected_failures branch from a8a4a77 to 9019cb0 Compare July 8, 2020 23:50
Copy link
Copy Markdown
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 with comments


def run(self, args):
return jsrun.run_js(self.filename, engine=self.engine, args=args, stderr=PIPE, full_output=True, assert_returncode=None)
return jsrun.run_js(self.filename, engine=self.engine, args=args, stderr=PIPE, full_output=True, assert_returncode=jsrun.NON_ZERO)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we don't test the benchmarks on CI, are you sure this is right? I'm surprised this expects a failure.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'm changed to expect success. All the other benchmark runners seems to expect success so I don't see why chreep wouldn't. I'm basically removing the ability to be ambivalent about this, so hopefully its easy enough to make sure the cheerp returns zero from these tests.

@sbc100 sbc100 merged commit bda3c1c into master Jul 9, 2020
@sbc100 sbc100 deleted the assert_on_expected_failures branch July 9, 2020 02:09
sbc100 added a commit to sbc100/emscripten that referenced this pull request Apr 14, 2026
- Remove unnecessary command line flags
- Check for test output
- Return 0 from test (otherwise we don't actually detect asan failures).
- Restore final checks to before emscripten-core#11578
sbc100 added a commit that referenced this pull request Apr 14, 2026
- Remove unnecessary command line flags
- Check for test output
- Return 0 from test (otherwise we don't actually detect asan failures).
- Restore final checks to before #11578
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