Clean up test infrastructure by Hixie · Pull Request #41880 · flutter/flutter

Hixie · 2019-10-03T00:34:18Z

Clean up cirrus.yaml
Clean up test.dart
Remove per-test timeout configuration in flutter_tools
Other random cleanup

Hixie · 2019-10-04T03:39:16Z

Most of the failures seem to be because I messed up the RAM requirements. I'm going to minimize the RAM requirements on all the shards and then increase them until each shard works.

codecov · 2019-10-05T01:36:00Z

Codecov Report

Merging #41880 into master will decrease coverage by 0.99%.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           master   #41880    +/-   ##
========================================
- Coverage   60.89%   59.89%    -1%     
========================================
  Files         194      194            
  Lines       18919    18919            
========================================
- Hits        11520    11331   -189     
- Misses       7399     7588   +189

Flag	Coverage Δ
#flutter_tool	`59.89% <ø> (-1%)`	⬇️

Impacted Files	Coverage Δ
packages/flutter_tools/lib/src/ios/devices.dart	`13.88% <0%> (-61.51%)`	⬇️
packages/flutter_tools/lib/src/ios/mac.dart	`26.33% <0%> (-27.9%)`	⬇️
packages/flutter_tools/lib/src/services.dart	`0% <0%> (-24.4%)`	⬇️
packages/flutter_tools/lib/src/base/build.dart	`59.66% <0%> (-15.97%)`	⬇️
packages/flutter_tools/lib/src/convert.dart	`58.33% <0%> (-8.34%)`	⬇️
packages/flutter_tools/lib/src/version.dart	`93.23% <0%> (-1.94%)`	⬇️
packages/flutter_tools/lib/src/context_runner.dart	`67.24% <0%> (-1.73%)`	⬇️
packages/flutter_tools/lib/src/mdns_discovery.dart	`82.53% <0%> (-1.59%)`	⬇️
packages/flutter_tools/lib/src/cache.dart	`48.47% <0%> (-0.71%)`	⬇️
packages/flutter_tools/lib/src/ios/xcodeproj.dart	`89.14% <0%> (-0.58%)`	⬇️
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 315471b...3583a98. Read the comment docs.

fkorotkov · 2019-10-06T19:57:15Z

I this empty container block messes the execution. Will add a check to the parser in the meantime.

Hixie · 2019-10-07T02:41:21Z

Aah! I was wondering if that would break things. It seemed like it should work, based on the YAML spec and the lack of error message about parse errors. :-)

I'll fix it.

tvolkert · 2019-10-16T18:51:25Z

Should _runFlutterTest() always just do a pass with --track-widget-creation and a pass with --no-track-widget-creation?

that could get pretty expensive. What bugs are you hoping to catch?

I was just reacting to the fact that we seem to be calling it with both of those flags in many places already.

some places, yeah. intentionally not all though.

tvolkert · 2019-10-16T18:52:50Z

If we ever add a new directory that matches packages/flutter/test/**/widgets, its tests will silently never be run...

unless i misunderstand, i don't believe that's an issue; recursive is false

Ah, I missed the recursive:false - nevermind.

tvolkert · 2019-10-16T18:54:34Z

Can you make sure that this cleanup will yield Cirrus tests that pass on dev, beta, and stable? Right now, we have tests that are guaranteed to fail on those channels (e.g. because Flutter for web is disabled on non-master).

I'm not sure how to test that short of actually doing a release.

You could temporarily (with no intention of submitting) edit the tools logic for what channel you're on to synthesize fake runs of dev|beta|stable. I leave it up to you on whether that churn of git push, wait, repeat is worth it.

tvolkert · 2019-10-16T18:56:25Z

Why shuffle it at all if we're going to hard code the seed?

I'm not sure what Ian's motivation was, but I like this change because randomizing should break up clusters of big tests making sharding more effective.

Might be worth a comment explaining why this is done.

yeah you want to shuffle so that the average cost-per-file is uniform.
i'll add a comment.

yjbanov · 2019-10-16T19:09:45Z

Also if changes include 'packages/flutter_web_plugins/***'

tvolkert

LGTM

tvolkert · 2019-10-17T06:32:31Z

Where is $CIRRUS_USER_COLLABORATOR set?

If it's set by Cirrus, is there a documentation page we can link to at the top of this YAML file?

Here is a link to all variable set by Cirrus: https://cirrus-ci.org/guide/writing-tasks/#environment-variables

tvolkert · 2019-10-17T06:34:52Z

Comment why you chose CPU:3

(and below)

tvolkert · 2019-10-17T06:47:00Z

It looks like we can probably remove key now (it looks like we never set $SHARD or $SUBSHARD anymore).

I'll add a comment. It's useful when doing local testing so you can run a whole shard (or a specific shard+subshard) without having to figure out the corresponding task name.

tvolkert · 2019-10-17T06:48:16Z

We were doing some gymnastics before to restore the values of these env variables after the tests were done. Is that not necessary?

Not as far as I can tell. Nothing runs after.

tvolkert · 2019-10-17T06:51:52Z

Should these not use backslashes?

I guess not? These are unchanged from before.

tvolkert · 2019-10-17T06:59:02Z

It looks like we run this for every MacOS task. Should we just put it in the setup_script that pertains to all tasks?

It has to run in the same shell as the script that uses a lot of files.

tvolkert · 2019-10-17T07:00:48Z

This looks SOOO much better. Note, however, that I didn't check to make sure that this PR doesn't accidentally drop any shards that we were running before.

tvolkert · 2019-10-17T07:09:36Z

You can say recursive: true and not need the existsSync() check

Interesting. Done.

Looks like you were wrong. :-)
https://cirrus-ci.com/task/6550651382792192?command=main#L104

It happens to the best of us. I even wrote a test that shows what the behavior is 😄

tvolkert · 2019-10-17T07:11:14Z

I wish enums could have properties & methods like they can in Java.

Hixie · 2019-10-18T00:00:13Z

cc @tvolkert @blasten this block is new, it's the result of merging with @blasten's recent changes

Basically I took the same approach as with the web tests (though sadly it's different enough that I couldn't reuse code).

This reverts commit 1781d5c.

See flutter#41880 for history.

See #41880 for history.

This reverts commit 1781d5c.

See flutter#41880 for history.

fluttergithubbot added c: contributor-productivity Team-specific productivity, code health, technical debt. tool Affects the "flutter" command-line tool. See also t: labels. labels Oct 3, 2019

googlebot added the cla: yes label Oct 3, 2019

Hixie force-pushed the cirrus.yaml branch 3 times, most recently from dfb1273 to 5d022d7 Compare October 4, 2019 00:21

Hixie mentioned this pull request Oct 4, 2019

Windows testing capacity is too low (Windows pre-commit tests disabled on cirrus) #41941

Closed

Hixie force-pushed the cirrus.yaml branch 3 times, most recently from 20dc493 to cc0689a Compare October 5, 2019 01:05

fkorotkov reviewed Oct 6, 2019

View reviewed changes

Hixie force-pushed the cirrus.yaml branch 11 times, most recently from 5af979c to 9f34245 Compare October 11, 2019 01:58

Hixie mentioned this pull request Oct 11, 2019

Shard Mac & Windows Cirrus jobs so execution time is limited to 15 mins. #42457

Closed

Hixie force-pushed the cirrus.yaml branch 3 times, most recently from 50b3a2f to 4fa532b Compare October 11, 2019 05:15

Hixie marked this pull request as ready for review October 11, 2019 18:18

Hixie force-pushed the cirrus.yaml branch 3 times, most recently from 0a907b2 to 0467e0b Compare October 16, 2019 01:31

tvolkert reviewed Oct 16, 2019

View reviewed changes

yjbanov reviewed Oct 16, 2019

View reviewed changes

Comment thread .cirrus.yml Outdated

Copy link
Copy Markdown

Contributor

yjbanov Oct 16, 2019

Choose a reason for hiding this comment

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

Also if changes include 'packages/flutter_web_plugins/***'

fkorotkov mentioned this pull request Oct 16, 2019

Use Cirrus credits for billing. #42867

Merged

8 tasks

Hixie force-pushed the cirrus.yaml branch 4 times, most recently from 91ac939 to 7536502 Compare October 16, 2019 23:33

tvolkert approved these changes Oct 17, 2019

View reviewed changes

Hixie force-pushed the cirrus.yaml branch 2 times, most recently from 9a2d736 to c69e00e Compare October 17, 2019 23:49

Hixie mentioned this pull request Oct 17, 2019

Reenable the dartdocs benchmark tracking #42882

Merged

Hixie force-pushed the cirrus.yaml branch from c69e00e to a47b650 Compare October 17, 2019 23:56

Hixie commented Oct 18, 2019

View reviewed changes

Hixie force-pushed the cirrus.yaml branch from a47b650 to 740b405 Compare October 18, 2019 00:21

Clean up test infrastructure

3583a98

Hixie force-pushed the cirrus.yaml branch from 740b405 to 3583a98 Compare October 18, 2019 00:37

Hixie merged commit 1781d5c into flutter:master Oct 18, 2019

jonahwilliams pushed a commit that referenced this pull request Oct 18, 2019

Revert "Clean up test infrastructure (#41880)"

c90559c

This reverts commit 1781d5c.

jonahwilliams mentioned this pull request Oct 18, 2019

Revert "Clean up test infrastructure" #42982

Merged

jonahwilliams pushed a commit that referenced this pull request Oct 18, 2019

Revert "Clean up test infrastructure (#41880)" (#42982)

1982a5c

This reverts commit 1781d5c.

Hixie added a commit to Hixie/flutter that referenced this pull request Oct 18, 2019

Clean up test infrastructure

472c429

See flutter#41880 for history.

Hixie mentioned this pull request Oct 18, 2019

Clean up test infrastructure #43030

Merged

Hixie added a commit that referenced this pull request Oct 18, 2019

Clean up test infrastructure (#43030)

124dc66

See #41880 for history.

Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Nov 26, 2019

Clean up test infrastructure (flutter#41880)

4c5d9e0

Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Nov 26, 2019

Revert "Clean up test infrastructure (flutter#41880)" (flutter#42982)

5fcd7f9

This reverts commit 1781d5c.

Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Nov 26, 2019

Clean up test infrastructure (flutter#43030)

502eda9

See flutter#41880 for history.

github-actions Bot locked as resolved and limited conversation to collaborators Aug 3, 2021

Conversation

Hixie commented Oct 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Hixie commented Oct 4, 2019

Uh oh!

codecov Bot commented Oct 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hixie commented Oct 7, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yjbanov Oct 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tvolkert left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Hixie commented Oct 3, 2019 •

edited

Loading

codecov Bot commented Oct 5, 2019 •

edited

Loading

yjbanov Oct 16, 2019 •

edited

Loading