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

Add test for profile mode diagnostics #17492

Merged
ferhatb merged 5 commits into from
May 15, 2020
Merged

Add test for profile mode diagnostics #17492

ferhatb merged 5 commits into from
May 15, 2020

Conversation

@ferhatb
Copy link
Contributor

@ferhatb ferhatb commented Apr 2, 2020

This PR requires Flutter PR flutter/flutter#53878 to land.

@ferhatb ferhatb requested a review from yjbanov April 2, 2020 23:07
@auto-assign auto-assign bot requested a review from gaaclarke April 2, 2020 23:07
@ferhatb ferhatb mentioned this pull request Apr 2, 2020
Merged
13 tasks
@yjbanov
Copy link
Contributor

yjbanov commented Apr 2, 2020

I think test should be in flutter/flutter, so that it breaks when the framework regresses again. Our e2e tests do not run on framework changes.

@ferhatb
Copy link
Contributor Author

ferhatb commented Apr 2, 2020

I think test should be in flutter/flutter, so that it breaks when the framework regresses again. Our e2e tests do not run on framework changes.

flutter tests run DDC. Not dart2js profile mode.

@yjbanov
Copy link
Contributor

yjbanov commented Apr 2, 2020

flutter tests run DDC. Not dart2js profile mode.

Sorry, I didn't mean literally flutter test. I meant a test similar to https://github.com/flutter/flutter/blob/a8b3d1b74fac5251c61c8a898cf7544fd77ddd16/dev/bots/test.dart#L582. Could also be an e2e test, just like this one, or something that cracks open the file and validates that certain symbols do not show up.

The code LGTM, so I'm approving. Leaving it up to you where to put this test, but it definitely won't run when someone breaks it. We'll only find out a breakage on a random engine PR, which could be unrelated to the breakage.

@nturgut
Copy link
Contributor

nturgut commented May 14, 2020

I'll merge this once the other PR is in. Thanks for the heads up.

@nturgut
Copy link
Contributor

nturgut commented May 15, 2020

I'll merge this once the other PR is in. Thanks for the heads up.

I'm waiting for this one to merge to merge my PR: flutter/flutter#57286

@ferhatb ferhatb merged commit a7270b2 into flutter:master May 15, 2020
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 16, 2020
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 16, 2020
@engine-flutter-autoroll
a7270b2 Add test for profile mode diagnostics (flutter/engine#17492)
c43012c
wandyers pushed a commit to wandyers/engine that referenced this pull request May 23, 2020
@ferhatb @wandyers
Add test for profile mode diagnostics (flutter#17492)
a5aa2e0 
* Add test for profile mode diagnostics

* revert cirrus change
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@gaaclarke gaaclarke Awaiting requested review from gaaclarke

1 more reviewer

@yjbanov yjbanov yjbanov approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ferhatb @yjbanov @nturgut @googlebot