[flutter_tools] rename two unit tests that were not actually being run on CI#98299
Conversation
flutter-dashboard
bot
added
c: contributor-productivity
dev/bots/analyze.dart
Outdated
| // detect files that contains calls to test(), testUsingContext(), and testWithoutContext() | ||
| final RegExp callsTestFunctionPattern = RegExp(r'(test\(.*\)|testUsingContext\(.*\)|testWithoutContext\(.*\))'); | ||
|
|
||
| await for (final File file in _allFiles(toolsTestPath, 'dart', minimumMatches: 42)) { |
There was a problem hiding this comment.
Remind me, what does minimumMatches do?
There was a problem hiding this comment.
oops, I need to change this. I put 42 there to get this working, lol
There was a problem hiding this comment.
I'm not sure the rationale (I asked in #hacker-tests) but what it effectively does is assert that it found more than this before returning. I upped this to 300 as we currently have 357 test files).
| @@ -1,60 +0,0 @@ | |||
| // Copyright 2014 The Flutter Authors. All rights reserved. | |||
There was a problem hiding this comment.
Can you just delete the one failing test instead of all of them?
There was a problem hiding this comment.
none of these are testing what they purport to be. The web tests are invalid (now that web is stable the feature flags are effectively ignored), and you should never be able to see multiple desktop platform validators at the same time (how they're shown is dependent on the platform OS).
Finally, when I tried to write a valid test, I couldn't figure out how to mock out the Platform with the testbed runner, as somehow the real platform was leaking through.
There was a problem hiding this comment.
Also, it was more than 1 failing: https://ci.chromium.org/b/8822482027585120017
jmagman
left a comment
jmagman
left a comment
There was a problem hiding this comment.
LGTM, we can fix up anything we missed in the regexes or file logic as we find problems.
christopherfujino
changed the title
Tests were removed in flutter#98299
3 participants
package:testwill only run tests that end in_test.dart. This PR adds a check in//flutter/dev/bots/analyze.dartthat validates all test files inpackage:flutter_toolsmatch this filename convention (see the test for the actual rules for detecting a "test file").I ended up deleting
packages/flutter_tools/test/general.shard/doctor.dartas they were mostly obsolete tests (web and windows desktop are now enabled by default on stable, regardless of feature flags) and were not written correctly.For
packages/flutter_tools/test/commands.shard/hermetic/custom_devices_test.dartI deleted 6 tests which were not passing (the others did pass).I filed a tracking issue to fix and re-enable the deleted custom devices tests: #98436