Add a E2E based performance test case#61509
Add a E2E based performance test case#61509fluttergithubbot merged 23 commits intoflutter:masterfrom CareF:e2e_trial
Conversation
|
A few things to address:
Before merge I need to run |
There was a problem hiding this comment.
This file probably also belongs to test_driver as this is unlikely to be used as a unit test. BTW, I guess we'll update WidgetTester to WidgetController in this file?
There was a problem hiding this comment.
I'm not expecting to use that demo very soon but if you think I should, maybe I'd better change this PR back to draft as that demo is far from use-able.
There was a problem hiding this comment.
And the reason I'm putting it in test/ rather than test_driver is that E2E recommends so (https://pub.dev/packages/e2e#test-locations), and the test file (cull_opacity_perf_e2e.dart) should be able to run using flutter test.
There was a problem hiding this comment.
Changing from WidgetTester to WidgetController for driverOps and setupOps for now, but this specific case it's not used.
There was a problem hiding this comment.
This file probably belongs to test_driver as this is unlikely to be used as a unit test.
|
I may also need to update the macrobenchmark/readme.md, which is also the case for #61388. That will be done on whichever of the two PR who lands later. |
liyuqian
left a comment
There was a problem hiding this comment.
I think this is good to go as soon as we minimize the duplicate code to make it a little easier to maintain in the future. Let's try to have this running in our device lab soon :)
| _countExceed(frameRasterizerTimeSorted, kBuildBudget), | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
Nit: when a function has a lot of arguments, Flutter usually put them under {} so it would be like the following
const FrameTimingSummarizer._({
this.frameBuildTime,
...
});
// Use it with argument name provided to reduce confusion
FrameTimingSummarizer._(
...,
averageFrameBuildTime: frameBuildTime.reduce(add) ~/ data.length,
...,
);
There was a problem hiding this comment.
{} means named arguments, and optional if not marked as @required, with default value if not assigned null. Here I don't think they should be optional. I added all @required.
Description
This PR reimplement
cull_opacity_perf__timeline_summarytest ascull_opacity_perf__e2e_summaryusing E2E, as a trial for running performance test using E2E that's host-independent in terms of driving the test.Related Issues
This is the first migration of performance test for go/flutter-nonhost
This is also a first step to #38329 for the new test case is self-driven.
Tests
The PR itself is adding a test case
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read Handling breaking changes.