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

[e2e] Use new FrameTiming constructor. #2914

Closed
wants to merge 3 commits into from
Closed

[e2e] Use new FrameTiming constructor. #2914

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
36f0271
[e2e] Use new FrameTiming constructor.
ditman Aug 10, 2020
e7cdc56
Add vsyncStart to the constructor, but the summarizer never uses it.
ditman Aug 10, 2020
ba0534a
Link to issue
ditman Aug 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/e2e/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.6.3+1

* Update test to use new `FrameTiming` constructor.

## 0.6.3

* Add customizable `flutter_driver` adaptor.
Expand Down
2 changes: 1 addition & 1 deletion packages/e2e/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: e2e
description: Runs tests that use the flutter_test API as integration tests.
version: 0.6.3
version: 0.6.3+1
homepage: https://github.com/flutter/plugins/tree/master/packages/e2e

environment:
Expand Down
9 changes: 8 additions & 1 deletion packages/e2e/test/frame_timing_summarizer_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,14 @@ void main() {
rasterTimes = rasterTimes.reversed.toList();
List<FrameTiming> inputData = <FrameTiming>[
for (int i = 0; i < 100; i += 1)
FrameTiming(<int>[0, buildTimes[i], 500, rasterTimes[i]]),
FrameTiming(
// TODO: Add vsyncStart to the FrameTimingSummarizer (https://github.com/flutter/flutter/issues/63415)
vsyncStart: 0,
buildStart: 0,
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the updates!
The new constructor introduced vsyncStart. Please use the following (or remove that 50+ and change the following expect accordingly:

        FrameTiming(
          vsyncStart: 0,
          buildStart: 50,
          buildFinish: 50+buildTimes[i],
          rasterStart: 500,
          rasterFinish: rasterTimes[i],
        ),

Copy link
Member Author

@ditman ditman Aug 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I've added vsyncStart: 0 and buildStart: 0. For now the object that this code is testing doesn't seem to use the vsyncStart. I've left a TODO, linked to this issue in the code to revisit this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good. I can handle this TODO.

buildFinish: buildTimes[i],
rasterStart: 500,
rasterFinish: rasterTimes[i],
),
];
FrameTimingSummarizer summary = FrameTimingSummarizer(inputData);
expect(summary.averageFrameBuildTime.inMicroseconds, 50500);
Expand Down