overall_experience_test passed in https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket.appspot.com/8847397369379619984/+/u/run_test.dart_for_tool_integration_tests_shard_and_subshard_4_4/stdout. Merging to deflake the tree.

Is there a discussion somewhere that I can read that describes why this helps?

Looking more closely I think this won't help, because it's still inside the same test which is where the timeout is applied. It should probably be moved to before the test.

Also I just noticed that there's a timeout on the test here, which we should remove. That timeout violates our style guide.

I thought the concern was that activating DevTools during flutter run could cause the flutter run execution to exceed the timeout used by the runFlutter function in this test.

The failure logs look like:

(matched /^An Observatory debugger and profiler on Flutter test device is available at: /)
(taking a long time...)
(process is not quitting, trying to send a "q" just in case that helps)
(a functional test should never reach this point)
stdout: Activating Dart DevTools...                                     
stdout: 
stdout: Application finished.
(process terminated with exit code 0)

The "taking a long time"/"process is not quitting" output is based on timeouts within runFlutter. Although the "process is not quitting" timeout appears to be set at 25 seconds, and it seems suspicious that this flutter run invocation would consume that much time even if it needs to activate DevTools.

Those timeouts are non-fatal (as all timeouts must be per our style guide), they're just thresholds for dumping more debugging information.

After the expectedMaxDuration timeout runFlutter sends a quit command to the flutter_tools process (which happened in the log taken from #82382).

There may be a race between this and the DevTools message that the test is waiting for.

Or possibly the DevTools message is not showing up as expected if the DevTools package needed to be activated on demand. I haven't seen that happen in local runs of this test though.

Yeah the intent of the expectedMaxDuration "send q" timeout is that that happens just before the end of the test just to clean things up. It was supposed to help debug things when they got stuck, and help prevent stray processes during debugging. Maybe we should remove that though, if it's causing more confusion than it's helping.