-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[e2e] add support to report extra information #2873
Conversation
It might be good to put some part of https://github.com/CareF/flutter/blob/e2e_trial/dev/benchmarks/macrobenchmarks/test/util.dart to E2E, but I'm not sure for that. |
2216192
to
182f6ad
Compare
df96795
to
e9bf7bb
Compare
Can you add some test(s) for this? |
@dnfield The test would require running on a device. I think I can add something to the |
You should be able to write some unit tests right? |
e9bf7bb
to
9d31c55
Compare
Currently we don't have any unit test for E2E, and I kind of believe it's because this package is all about adapting for running tests for different ways. But yeah it's probably the good point to start unit test here. |
@dnfield Unit test done. I'm a little bit proud for coming up a way to unit test the binding. |
1d3833a
to
f9eb6db
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with one nit.
* add extra info report
* add extra info report
Description
Added an extra field
extraInfo
inlib/common.dart
. Use case is to use the field to report performance. E.g. in flutter/flutter#61509, I'm migrating one existing test to an e2e based performance test. The use case is not complete because I still need to add other performance metrics but for a demo of the use case it's sufficient.I'm not naming it to performance because in some of our device lab test the reported value is not for performance.
Related Issues
Resolves flutter/flutter#61490
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?