Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[tools] Fix vm test requirement #6995

Merged
merged 2 commits into from
Jun 27, 2024

Conversation

stuartmorgan
Copy link
Contributor

The logic for handling Dart unit test test_on directives was incorrect, causing it to skip packages that required vm testing, even when run in vm mode. This PR:

  • Adds the missing tests for false negatives.
  • Reworks the logic to explicitly fail for anything that isn't one of the exact patterns we are expecting, to make it much harder to re-introduce a bug like this in the future.

The logic for handling Dart unit test `test_on` directives was
incorrect, causing it to skip packages that required vm testing, even
when run in vm mode. This PR:
- Adds the missing tests for false negatives.
- Reworks the logic to explictly fail for anything unrecognized, to make
  it much harder to re-introduce a bug like this in the future.
@stuartmorgan
Copy link
Contributor Author

Hopefully we won't find latent test failures that this was hiding :|

@tarrinneal
Copy link
Contributor

looks like there's more non-test test failures

@stuartmorgan
Copy link
Contributor Author

The package dependencies in flutter_migrate had become stale since we weren't running the tests and getting failures on rolls the way we did with other packages.

@stuartmorgan
Copy link
Contributor Author

Verified in CI output that this does fix the thing that caused us to find this in the first place:

packages/pigeon - ran

(It was incorrectly skipped before.)

@stuartmorgan stuartmorgan added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 26, 2024
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 27, 2024
Copy link
Contributor

auto-submit bot commented Jun 27, 2024

auto label is removed for flutter/packages/6995, due to - The status or check suite Mac_arm64 ios_platform_tests_shard_5 master has failed. Please fix the issues identified (or deflake) before re-applying this label.

@stuartmorgan stuartmorgan added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 27, 2024
Copy link
Contributor

auto-submit bot commented Jun 27, 2024

auto label is removed for flutter/packages/6995, due to - The status or check suite Mac_arm64 ios_platform_tests_shard_5 master has failed. Please fix the issues identified (or deflake) before re-applying this label.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 27, 2024
@stuartmorgan stuartmorgan added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 27, 2024
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 27, 2024
Copy link
Contributor

auto-submit bot commented Jun 27, 2024

auto label is removed for flutter/packages/6995, due to - The status or check suite Mac_arm64 ios_platform_tests_shard_5 master has failed. Please fix the issues identified (or deflake) before re-applying this label.

@stuartmorgan stuartmorgan added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 27, 2024
@stuartmorgan
Copy link
Contributor Author

The macOS infra flake is getting pretty bad :(

@auto-submit auto-submit bot merged commit 69e7fc1 into flutter:main Jun 27, 2024
74 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 27, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 27, 2024
@stuartmorgan stuartmorgan deleted the dart-unit-test-fix-skip branch June 27, 2024 16:15
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 27, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 28, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jun 28, 2024
flutter/packages@03f5f6d...412ec46

2024-06-28 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 3.25.6 to 3.25.10 (flutter/packages#6924)
2024-06-27 [email protected] [video_player] Exposes VideoPlayerWebOptions. (flutter/packages#6990)
2024-06-27 [email protected] [ci] Add Wasm compilation to all_packages web app. (flutter/packages#6981)
2024-06-27 [email protected] [interactive_media_ads] Updates `README` with a usage section and fix some interface docs (flutter/packages#6988)
2024-06-27 [email protected] [tools] Fix vm test requirement (flutter/packages#6995)
2024-06-27 [email protected] Update `web_benchmarks` package to properly support wasm. (flutter/packages#6970)
2024-06-27 [email protected] [pigeon] Implement equals for Java data classes (flutter/packages#6992)
2024-06-25 [email protected] Final refactor of `video_player_android` before `SurfaceProducer#setCallback`. (flutter/packages#6982)
2024-06-25 [email protected] [pigeon] Update testing and docs (flutter/packages#6984)
2024-06-25 [email protected] [various] Update flutter.dev links to more reliable destinations (flutter/packages#6963)
2024-06-25 [email protected] [interactive_media_ads] Adds initial Android implementation (flutter/packages#6733)
2024-06-25 [email protected] Roll Flutter from 6c06abb to e726eb4 (51 revisions) (flutter/packages#6987)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
victorsanni pushed a commit to victorsanni/flutter that referenced this pull request Jul 8, 2024
flutter/packages@03f5f6d...412ec46

2024-06-28 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 3.25.6 to 3.25.10 (flutter/packages#6924)
2024-06-27 [email protected] [video_player] Exposes VideoPlayerWebOptions. (flutter/packages#6990)
2024-06-27 [email protected] [ci] Add Wasm compilation to all_packages web app. (flutter/packages#6981)
2024-06-27 [email protected] [interactive_media_ads] Updates `README` with a usage section and fix some interface docs (flutter/packages#6988)
2024-06-27 [email protected] [tools] Fix vm test requirement (flutter/packages#6995)
2024-06-27 [email protected] Update `web_benchmarks` package to properly support wasm. (flutter/packages#6970)
2024-06-27 [email protected] [pigeon] Implement equals for Java data classes (flutter/packages#6992)
2024-06-25 [email protected] Final refactor of `video_player_android` before `SurfaceProducer#setCallback`. (flutter/packages#6982)
2024-06-25 [email protected] [pigeon] Update testing and docs (flutter/packages#6984)
2024-06-25 [email protected] [various] Update flutter.dev links to more reliable destinations (flutter/packages#6963)
2024-06-25 [email protected] [interactive_media_ads] Adds initial Android implementation (flutter/packages#6733)
2024-06-25 [email protected] Roll Flutter from 6c06abb to e726eb4 (51 revisions) (flutter/packages#6987)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: flutter_migrate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants