flutter / flutter

Flutter makes it easy and fast to build beautiful apps for mobile and beyond
https://flutter.dev
BSD 3-Clause "New" or "Revised" License
161.86k stars 26.57k forks source link

[test_runner] Consolidate runDartTest, runFlutterTest and runFlutterWebTest into a single function #145944

Open godofredoc opened 1 month ago

godofredoc commented 1 month ago

https://cs.opensource.google/flutter/flutter/+/master:dev/bots/test.dart has three functions with similar functionality. Most of the code of those three functions can be consolidated into a single one.

godofredoc commented 1 month ago

@goderbauer @christopherfujino if flutter test can run dart test tests would it make sense to use flutter test consistently across dev/bots/test.dart?

goderbauer commented 1 month ago

Scanning through test.dart it looks like these are the tests currently run with dart test:

I'll let @christopherfujino speak about the flutter_tools and conductor tests. ~For the bots and devicelab tests: If those run without any issues with flutter test I would be okay with simplifying our logic here and running them all with the same mechanism.~ (edit: see comment below)

christopherfujino commented 3 weeks ago

I think we should use dart test on packages that are not flutter apps. I don't know everything extra that flutter test does, but I know it's a lot, so I'd rather have less code to consider when debugging weird failures (also there might be performance overhead to the way flutter test launches the test, but not sure).

Finally, I suspect this might break when we fix: https://github.com/flutter/flutter/issues/91107 (cc @eliasyishak)

goderbauer commented 3 weeks ago

That's a good point, Chris. If we fix https://github.com/flutter/flutter/issues/91107 (which I hope we do at some point) most of these tests wouldn't run with flutter test anymore. So, we really shouldn't consolidate this.