Skip to content

Convert sample batch of browser tests to avoid REPORT_RESULT #12977

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 1 commit into from
Dec 11, 2020

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Dec 5, 2020

For simple browser tests we can just rely on the process
exit code being reported. I've added a new btest_exit
method so that tests can opt into this new behavior.

This allows test code to be identical whether running
in the browser or outside.

@sbc100 sbc100 force-pushed the avoid_report_result branch 3 times, most recently from 2f9167f to 8e8fbbe Compare December 5, 2020 19:32
@sbc100 sbc100 force-pushed the browser_test_warnings branch 2 times, most recently from a3fa14c to b646a78 Compare December 5, 2020 21:48
@sbc100 sbc100 force-pushed the avoid_report_result branch 2 times, most recently from 6e419ec to a6d05e2 Compare December 6, 2020 04:05
@sbc100 sbc100 force-pushed the browser_test_warnings branch from b646a78 to 921c5a3 Compare December 6, 2020 21:00
Base automatically changed from browser_test_warnings to master December 6, 2020 21:00
@sbc100 sbc100 force-pushed the avoid_report_result branch 3 times, most recently from dfb6f73 to 80b9d16 Compare December 9, 2020 02:11
@sbc100 sbc100 requested a review from kripken December 9, 2020 19:35
@sbc100 sbc100 force-pushed the avoid_report_result branch from 80b9d16 to 49988b5 Compare December 10, 2020 23:59
@sbc100 sbc100 force-pushed the avoid_report_result branch from b3e0a61 to d69c4a8 Compare December 11, 2020 00:26
For simple browser tests we can just rely on the process
exit code being reported.  I've added a new btest_exit
method so that tests can opt into this new behavior.

This allows test code to be identical whether running
in the browser or outside.
@sbc100 sbc100 force-pushed the avoid_report_result branch from 59a831b to 77cce4b Compare December 11, 2020 01:21
@sbc100
Copy link
Collaborator Author

sbc100 commented Dec 11, 2020

I ended up going for the enum approach as it made more sense to me in the end. What do you think?

@sbc100 sbc100 merged commit 3500c14 into master Dec 11, 2020
@sbc100 sbc100 deleted the avoid_report_result branch December 11, 2020 02:13
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.

enum lgtm

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