Added 'exclude' parameter to 'testWidgets()'. #86397
Conversation
|
Gold has detected about 1 new digest(s) on patchset 1. |
|
Can't one just not call the This seems to just add API surface without actually providing more power. Given the complexity of the |
We could, but it would mean that those tests aren't reported in any way if they are run under the condition that caused them to be skipped. This may be fine, but could lead to some confusion about the results. (Also, I am not that familiar with the underlying test runner, but @gspencergoog just added the ability to randomly shuffle the order of tests. In this case it would just not add them to the list that is shuffled, right Greg? or would this introduce an issue here?)
This would mean that the test is reported as a success when it didn't run.
While I understand not wanting to grow the API surface, this seems like a good tradeoff to me. It would give a declarative way to say that a test is intentionally skipped under some condition. |
|
Gold has detected about 5 new digest(s) on patchset 1. |
I actually had to revert that change, but I will reintroduce it when the test package is next released. In any case, it would just mean that the test wouldn't be included in the shuffled list, so no issue here. |
|
I really don't see the problem with reporting the test as successful if it didn't run, or not reporting them when they're skipped, to be honest. Does anyone really look at the precise numbers such that it would matter? Honestly I'd be perfectly fine with the entire test system being pass/fail, no numbers at all. I wish we could get rid of the |
Ah, I can see why you wouldn't be keen on this idea then While I respect that, I still see value in having structured tests with data that describe them to making it easier for maintenance and tooling. And if we are going to have this test structure it would be nice to have a way to tell intentional skips from temporary using the existing structure. |
|
I hear what you're saying. Is there a way we can add this feature to our own test infrastructure without adding the complexity to our API? I'm worried about how every new person learning how to use tests in Flutter is going to have to distinguish between (At the end of the day, I'm not too worried about us having a few dollars extra on our benchmark, I have to be honest. If that's the worst thing we're dealing with, we can call the project finished and move on to something else. :-) ) |
|
Skip vs exclude sounds like a worthwhile distinction, LGTM |
eb62bce
into
flutter:master
Part of #86396.
Adds a new
excludeparameter totestWidgetsthat behaves the same asskipbut is used to mark a test that should always be skipped under the condition as it isn't designed that way.skipis still used to mark tests that are just temporarily shut off to keep the tree open.Pre-launch Checklist
///).