Fix surface-size state leak in material/paginated_data_table_test (framework shuffle-all 3/n)

[gnprice]

Fixes part of #85160. This comes after #123751 and #123806 as part of removing no-shuffle tags from all tests in the framework.

The tests in this file had a state leak because although many of them were scrupulously resetting the surface size, two tests were missing that step:

• "PaginatedDataTable with optional column checkbox"
• "PaginatedDataTable arrowHeadColor set properly"

We could add a line addTearDown(() => binding.setSurfaceSize(null)); to those two tests. But it seems like we'll inevitably keep having bugs of this form — and so will any other flutter_test users that use this setSurfaceSize feature — unless the feature arranges to automatically either check that the state got reset, or just reset it.

And then once we're automatically detecting that the state wasn't reset, it doesn't seem like there's anything to be gained by making the author of the individual test take care of it. This isn't a situation where there's any ambiguity about how to handle the reset, or where the un-reset data could be a sign of a broader bug; the thing the test author would do is always to just reset the state to the default anyway. So take care of it automatically.

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
• I signed the CLA.
• I listed at least one issue that this PR fixes in the description above.
• I updated/added relevant documentation (doc comments with ///).
• I added new tests to check the change I am making, or this PR is test-exempt.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[gnprice]

To check there weren't other ordering dependencies lurking in this file, I ran it with 100 different seeds. With only 18 tests in the file, an ordering-based flake would be expected to have a rate of at least 1/18 > 5.5%, so the data is enough to reject that with p<.004.

[goderbauer]

@pdblasi-google Can you take a look at this one since you've been thinking already a bit about resetting test state to avoid leaks?

[pdblasi-google]

@gnprice

This is good work, but I'd like to tackle this problem from another direction. For getting the no-shuffle off of the paginated_data_table_test I'd prefer if you added the addTearDown(() => binding.setSurfaceSize(null)); to the two tests that need it for the time being.

Thanks for bringing my attention to the setSurfaceSize method. I've seen it around and it was throwing up red flags, but this PR gave me a reason to deep dive on it. I've written up #123881 with more details, but the TLDR; is that setSurfaceSize really should be TestFlutterView.physicalSize at this point as they should be equivalent.

If all instances of setSurfaceSize are instead TestFlutterView.physicalSize, then as part of #121917 they will get the "enforce reset instead of fail" behavior planned in that ticket. I'd like as much of the automatic resetting to be part of that ticket as possible for the following reasons:

1. Instead of spreading out any issues with automatic resetting over multiple versions, there will be a single version increment with a warning and a migration guide on how to handle potential issues caused by the automatic resets
2. Tests that were depending on the leaked state (intentionally or not) will have a cleaner migration path with that ticket as they'll be able to retain the same behavior (without leaks) using the setUpWidgets function from that ticket

[gnprice]

@pdblasi-google OK, that plan makes sense to me.

I'll rework this PR to just locally fix those tests using addTearDown, and perhaps add a pointer to [TestFlutterView.physicalSize] in the setSurfaceSize doc.

[gnprice]

Done; PTAL.

[pdblasi-google]

LGTM!