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

Conversation

@jason-simmons
Copy link
Member

This test has been flaky on LUCI recently.

See flutter/flutter#65026

sk_sp<SkData> screenshot_data = screenshot_future.get().data;
if (!reference_data->equals(screenshot_data.get())) {
LogSkData(reference_data, "reference");
LogSkData(reference_data, "screenshot");
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
LogSkData(reference_data, "screenshot");
LogSkData(screenshot_data, "screenshot");

Copy link
Member Author

Choose a reason for hiding this comment

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

done

LogSkData(reference_data, "reference");
LogSkData(reference_data, "screenshot");
}
ASSERT_TRUE(reference_data->equals(screenshot_data.get()));
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can we just do something like ASSERT_TRUE(false) in the if block above to avoid doing the comparison twice?

Copy link
Member Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

Choose a reason for hiding this comment

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

We should then drop this line

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@jason-simmons jason-simmons force-pushed the bug_65026_log branch 2 times, most recently from 360be7b to 1c08a20 Compare September 1, 2020 23:03
This test has been flaky on LUCI recently.

See flutter/flutter#65026
Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

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

LGTM

@jason-simmons jason-simmons added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Sep 3, 2020
@jason-simmons jason-simmons merged commit d368b87 into flutter:master Sep 3, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 4, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants