[e2e] Add new e2e_driver for handling response data and performance watcher #2906
Conversation
|
@liyuqian Our previous attempts to reuse |
packages/e2e/lib/e2e_perf.dart
Outdated
| binding.removeTimingsCallback(watcher); | ||
| // TODO(CareF): determine if it's running on firebase and report metric online | ||
| final FrameTimingSummarizer frameTimes = FrameTimingSummarizer(frameTimings); | ||
| binding.reportData = <String, dynamic>{'performance': frameTimes.summary}; |
There was a problem hiding this comment.
question: Is binding.reportData eventually picked up in driver response?
There was a problem hiding this comment.
My original version in e2e_driver.dart particularly picked the performance value to write to file but I realized that e2e_driver.dart is probably used in a broader context, e.g. this reportData feature was introduced partly as an infra preparation for getting timeline and reporting it to the host in an e2e test.
There was a problem hiding this comment.
I see, thanks for the explanation!
packages/e2e/lib/e2e_perf.dart
Outdated
| binding.removeTimingsCallback(watcher); | ||
| // TODO(CareF): determine if it's running on firebase and report metric online | ||
| final FrameTimingSummarizer frameTimes = FrameTimingSummarizer(frameTimings); | ||
| binding.reportData = <String, dynamic>{'performance': frameTimes.summary}; |
There was a problem hiding this comment.
I see, thanks for the explanation!
| /// ``` | ||
| Future<void> e2eDriver({ | ||
| Duration timeout = const Duration(minutes: 1), | ||
| ResponseDataCallback responseDataCallback = writeResponseData, |
There was a problem hiding this comment.
Default to write response.data into e2e_perf_summary.json seems a little strange as it doesn't necessarily have perf summary data. Shall we default to not write any file, and provide a perfResponseCallback that writes response.data['performance'] into e2e_perf_summary.json? That seems a little more helpful in updating your current code in https://github.com/flutter/flutter/pull/61509/files.
There was a problem hiding this comment.
I probably should change the file name to some more general ones. Getting performance entry is just one more line like in https://github.com/flutter/flutter/pull/62064/files#diff-73e66281b14c792afcdcd20acb9edd92R11 while I'm imagining this e2eDriver will be used in a broader context, e.g. reporting timeline as in flutter/flutter#58789 , so I'm trying to make this function useful independent of the performance test context while still being useful in my project.
There was a problem hiding this comment.
As for by default not writing anything, writeResponseData will look like an unused method. Maybe I should add an example to e2e as for demo the usage of this PR. If we decide to go that direction I'll open a new PR after this lands for that.
Description
These are utilities for implementing e2e-based performance test. e.g.flutter/flutter#61509
These codes will be shared among most of such test cases.
Related Issues
flutter/flutter#58789
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) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?