Made sure no test() declaration happens asynchronously in fragment shader tests#182306
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors a set of fragment shader tests to address an issue where test() declarations were made asynchronously. The change introduces a group() and moves the asynchronous initialization of ImageComparer into a setUpAll() block. This ensures that all test() declarations are made synchronously, which is the correct pattern for Dart tests.
|
Can you explain how this change fixes the issue? Is Or is it not supposed to be called/awaited after earlier Other tests call/await flutter/engine/src/flutter/testing/dart/gpu_test.dart Lines 148 to 149 in 0b3185c flutter/engine/src/flutter/testing/dart/canvas_test.dart Lines 175 to 176 in 0b3185c I think we should make sure this test is consistent with all these other test files.
To me it does seem a bit weird to have this setup code outside of |
Sure. Check out the error message linked in the issue. What it is saying is you can't call the function This generates a race condition where depending on how long
Any
Exactly. |
|
Approved to not block. But I think we should follow-up to change all the other test files to put |
|
This is a nasty gotcha, it would be nice to have a lint for this. |
Filed dart-lang/test#2593 |
|
autosubmit label was removed for flutter/flutter/182306, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label. |
|
this should improve if not completely address #182261 |
…ragment shader tests (flutter/flutter#182306)
…ragment shader tests (flutter/flutter#182306)
…ragment shader tests (flutter/flutter#182306)
…ragment shader tests (flutter/flutter#182306)
…ragment shader tests (flutter/flutter#182306)
…ragment shader tests (flutter/flutter#182306)
…ragment shader tests (flutter/flutter#182306)
…ragment shader tests (flutter/flutter#182306)
…ragment shader tests (flutter/flutter#182306)
…ragment shader tests (flutter/flutter#182306)
…ragment shader tests (flutter/flutter#182306)
…ragment shader tests (flutter/flutter#182306)
…ragment shader tests (flutter/flutter#182306)
…ragment shader tests (flutter/flutter#182306)
…ragment shader tests (flutter/flutter#182306)
…ragment shader tests (flutter/flutter#182306)
…ader tests (flutter#182306) fixes flutter#182305 test exempt: is a test ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
…ader tests (flutter#182306) fixes flutter#182305 test exempt: is a test ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
fixes #182305
test exempt: is a test
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.