Skip to content

Failed assertions when #2498

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

Closed
bkonyi opened this issue May 15, 2025 · 6 comments · Fixed by #2499
Closed

Failed assertions when #2498

bkonyi opened this issue May 15, 2025 · 6 comments · Fixed by #2499

Comments

@bkonyi
Copy link
Contributor

bkonyi commented May 15, 2025

When attempting to do a pub package roll into the Flutter framework, we've started encountering failures due to assertions added in 935b8b0.

In particular, we're seeing this assertion fail in all three tests referencing flutter/dev/automated_tests/flutter_test/trivial_widget_test.dart in flutter/packages/flutter_tools/test/integration.shard/test_test.dart:

              package:test_api/src/backend/group.dart': Failed assertion: line 73 pos 12: '<optimized out>': is not true.
              dart:core-patch/errors_patch.dart 67:4                _AssertionError._doThrowNew
              dart:core-patch/errors_patch.dart 49:5                _AssertionError._throwNew
              package:test_api/src/backend/group.dart 73:12         new Group
              package:test_api/src/backend/group.dart 95:12         Group.filter
              package:test_core/src/runner/runner_suite.dart 66:26  RunnerSuite.filter
              package:test_core/src/runner.dart 283:34              Runner._loadSuites.<fn>.<fn>
              package:test_core/src/runner/load_suite.dart 185:26   LoadSuite.changeSuite.<fn>.<fn>
              dart:async/zone.dart 1525:13                          _rootRun
              dart:async/zone.dart 1422:19                          _CustomZone.run
              dart:async/zone.dart 1321:7                           _CustomZone.runGuarded
              package:test_core/src/runner/load_suite.dart 184:12   LoadSuite.changeSuite.<fn>
              dart:async/zone.dart 1538:47                          _rootRunUnary
              dart:async/zone.dart 1429:19                          _CustomZone.runUnary
              dart:async/future_impl.dart 951:45                    Future._propagateToListeners.handleValueCallback
              dart:async/future_impl.dart 980:13                    Future._propagateToListeners
              dart:async/future_impl.dart 714:7                     Future._complete
              dart:async/future_impl.dart 116:12                    _SyncCompleter.complete
              package:test_core/src/runner/load_suite.dart 108:19   new LoadSuite.<fn>.<fn>

Full logs are here.

I'm going to pin the dependency on package:test_api to 0.7.4 prevent the latest change from being pulled in.

FYI @DanTup @jakemac53

bkonyi added a commit to flutter/flutter that referenced this issue May 15, 2025
@jakemac53
Copy link
Contributor

@DanTup can you take a look?

@DanTup
Copy link
Contributor

DanTup commented May 15, 2025

Yep, looking... I guess these asserts are the ones to ensure we had consistent parents when cloning the groups/tests.

@DanTup
Copy link
Contributor

DanTup commented May 15, 2025

@bkonyi btw, it looks like this is already pinned in Flutter?

https://github.com/flutter/flutter/blob/d69edc303b494cec608acd1e937c844499448a40/packages/flutter_tools/pubspec.yaml#L65

Did the attempted roll change this to get the broken version, or is something else up too?

Edit: nm, I found your PR flutter/flutter#168916 that updated this - I'll debug using that PR!

@DanTup
Copy link
Contributor

DanTup commented May 15, 2025

@bkonyi I don't suppose you can repro this locally?

I'm struggling with your PR (with the fix taken out) either running the Flutter tests, or the sample test directly.

I believe I know what the issue is - I think we're missing clones of setUpAll/tearDownAll here:

https://github.com/dart-lang/test/blob/b9c59ea01ab0c055d120fd6542af663704c16938/pkgs/test_api/lib/src/backend/group.dart#L100C22-L100C33

I'm working on a fix, but it'd be good to verify it solves the problem here by being able to repro the issue without having to release etc.

Edit: nm, dart --enable-asserts test works!

DanTup added a commit to DanTup/test that referenced this issue May 15, 2025
When groups are filtered using `filter()` and not `onPlatform`, we would clone the group and tests without also cloning `setUpAll`/`tearDownAll`, which led to an assertion failure when trying to set the parents.

This change ensures we clone tests in `filter()` like we do in `forPlatform()`.

Fixes dart-lang#2498
@DanTup
Copy link
Contributor

DanTup commented May 15, 2025

I was able to repro locally with Flutter by setting $env:FLUTTER_TOOL_ARGS="--enable-asserts" (which I think the CI/tests effectively do here), and confirmed that the issue goes away with the fix I'm working on at #2499

Image

jakemac53 pushed a commit that referenced this issue May 19, 2025
When groups are filtered using `filter()` and not `onPlatform`, we would clone the group and tests without also cloning `setUpAll`/`tearDownAll`, which led to an assertion failure when trying to set the parents.

This change ensures we clone tests in `filter()` like we do in `forPlatform()`.

Fixes #2498
DanTup added a commit to DanTup/flutter that referenced this issue May 21, 2025
These packages were recently pinned (in flutter#168916) due to a bug (dart-lang/test#2498).

That bug was fixed and new versions of the test packages published. This change unpins and lets them upgrade to the fixed version.
@DanTup
Copy link
Contributor

DanTup commented May 21, 2025

I missed that @jakemac53 had already published these fixes, so anyone with failures should see them fixed now (or if had pinned to the previous version, can unpin).

@bkonyi I've started on a PR to unpin the packages in Flutter to verify everything passes this time (flutter/flutter#169198).

github-merge-queue bot pushed a commit to flutter/flutter that referenced this issue May 22, 2025
These packages were recently pinned (in
#168916) due to a bug
(dart-lang/test#2498).

That bug was fixed and new versions of the test packages published. This
change unpins and lets them upgrade to the fixed version.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants