Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@yjbanov
Copy link
Contributor

@yjbanov yjbanov commented Jul 12, 2021

Rename Pipeline.start to Pipeline.run and have it rethrow the exception. This makes sure that felt exits with a non-zero code when there's an exception in the infra or test harnesses. The exception swallowing logic moved to PipelineWatcher, so only the watch mode is affected.

Fixes flutter/flutter#85387

@yjbanov yjbanov requested a review from mdebbar July 12, 2021 20:55
@flutter-dashboard flutter-dashboard bot added the platform-web Code specifically for the web engine label Jul 12, 2021
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

await pipeline.run();
_pipelineSucceeded(runCount);
} catch(error, stackTrace) {
_pipelineFailed(error, stackTrace);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add comment explaining why a rethrow isn't needed here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@yjbanov yjbanov merged commit bde2f52 into flutter:master Jul 13, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 14, 2021
moffatman pushed a commit to moffatman/engine that referenced this pull request Aug 5, 2021
* [web] make Pipeline.run throw
naudzghebre pushed a commit to naudzghebre/engine that referenced this pull request Sep 2, 2021
* [web] make Pipeline.run throw
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes needs tests platform-web Code specifically for the web engine

Projects

None yet

Development

Successfully merging this pull request may close these issues.

web engine integration tests failing, but reporting success

2 participants