Skip to content

Reverts "Fail tests on exceptions raised after test completed (#144706)" #144970

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.

Sign up for GitHub

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

Jump to bottom
Merged
auto-submit merged 1 commit into from
Mar 11, 2024
Merged

Reverts "Fail tests on exceptions raised after test completed (#144706)" #144970

auto-submit merged 1 commit into from
Mar 11, 2024

Conversation

@auto-submit
Copy link
Contributor

@auto-submit auto-submit bot commented Mar 11, 2024

Reverts: #144706

Initiated by: gspencergoog

Reason for reverting: This has broken the tree because some tests are still failing post completion. This particular one looks like it might have to do with a gold image not existing.

Original PR Author: goderbauer

Reviewed By: {Piinks}

This change reverts the following previous change:
A test was failing silently because of this (see #144353 and fixed in #144709). The failure went undetected for months. Ideally, this should have been a regular non-silent failure. This change makes that so. package:test can properly handle reported exceptions outside of test cases. With this change, the test fails as follows:

00:03 +82: Smoke test material/color_scheme/dynamic_content_color.0.dart
══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following assertion was thrown running a test (but after the test had completed):
setState() called after dispose(): _DynamicColorExampleState#1cd37(lifecycle state: defunct, not
mounted)
This error happens if you call setState() on a State object for a widget that no longer appears in
the widget tree (e.g., whose parent widget no longer includes the widget in its build). This error
can occur when code calls setState() from a timer or an animation callback.
The preferred solution is to cancel the timer or stop listening to the animation in the dispose()
callback. Another solution is to check the "mounted" property of this object before calling
setState() to ensure the object is still in the tree.
This error might indicate a memory leak if setState() is being called because another object is
retaining a reference to this State object after it has been removed from the tree. To avoid memory
leaks, consider breaking the reference to this object during dispose().

When the exception was thrown, this was the stack:
#0      State.setState.<anonymous closure> (package:flutter/src/widgets/framework.dart:1167:9)
#1      State.setState (package:flutter/src/widgets/framework.dart:1202:6)
#2      _DynamicColorExampleState._updateImage (package:flutter_api_samples/material/color_scheme/dynamic_content_color.0.dart:191:5)
<asynchronous suspension>
════════════════════════════════════════════════════════════════════════════════════════════════════
00:03 +81 -1: Smoke test material/context_menu/context_menu_controller.0.dart
00:03 +81 -1: Smoke test material/color_scheme/dynamic_content_color.0.dart [E]
  Test failed. See exception logs above.
  The test description was: Smoke test material/color_scheme/dynamic_content_color.0.dart
  
  This test failed after it had already completed.
  Make sure to use a matching library which informs the test runner
  of pending async work.

@auto-submit auto-submit bot added the revert of Bot Only: Tracking label for bot. Tracks new revert of pull requests. label Mar 11, 2024
@github-actions github-actions bot added a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. labels Mar 11, 2024
@gspencergoog gspencergoog requested a review from goderbauer March 11, 2024 23:31
@auto-submit auto-submit bot merged commit 49f45d2 into master Mar 11, 2024
@auto-submit auto-submit bot deleted the revert_91cccc8e6b83ada2dcffe266ff4a6d3c8d0ea582 branch March 11, 2024 23:31
@gspencergoog
Copy link
Contributor

gspencergoog commented Mar 11, 2024

Errors that precipitated the revert:

https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8753720749767757601/+/u/run_test.dart_for_web_tests_shard_and_subshard_1/stdout

The error text was:

04:56 +914 ~82: test/material/input_chip_test.dart: Delete button is visible on disabled InputChip                                                                                                     
══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following TestFailure was thrown running a test (but after the test had completed):
Expected: one widget whose rasterized image matches golden image
"input_chip.disabled.delete_button.png"
  Actual: _TypeWidgetFinder:<Found 1 widget with type "RawChip": [
            StatefulElement#cd39c(DEFUNCT)(no widget),
          ]>
   Which: SkiaException: Skia Gold received an unapproved image in post-submit
          testing. Golden file images in flutter/flutter are triaged
          in pre-submit during code review for the given PR.

          Visit https://flutter-gold.skia.org// to view and approve
          the image(s), or revert the associated change. For more
          information, visit the wiki:
          https://github.com/flutter/flutter/wiki/Writing-a-golden-file-test-for-package:flutter

          Debug information for Gold --------------------------------
          stdout: Given image with hash 355e4280d8ca02fbdcf5fe321f0af129 for test
material.input_chip.disabled.delete_button
          Expectation for test: 317bd7e3a5400ffec160c3a10b2ac25c (negative)
          Expectation for test: 5ff98f86bddd64e527a3c2e26fd26331 (positive)
          Expectation for test: 696b4bbc4381900c7b4b2464b6de30f9 (positive)
          Expectation for test: dba77e2843dfcb0d2ec5803a3665f2d1 (positive)
          Expectation for test: 0db619c055930deebebf1340baab90e5 (positive)
          Expectation for test: 2ce5a266a7f6f622708eabfd707a7437 (positive)
          Expectation for test: 3a1a7b2a497a4c9184cfe7df37218b6a (positive)
          Expectation for test: 866566f653617ed95301e9083ee9c451 (positive)
          Expectation for test: 9fc8003bfff820a37313963b517b6b33 (positive)
          Expectation for test: d336da7e697057ee8af6ca8f978e176e (positive)
          Expectation for test: 155a76b26e049ebc408f50458bc714d7 (positive)
          Expectation for test: 3523e9437e2da46e071812e0c97158f1 (positive)
          Expectation for test: 355e4280d8ca02fbdcf5fe321f0af129 (negative)
          Expectation for test: 863bc286a8cb7637706c48a3a70ea0ec (positive)
          Expectation for test: a655c4207515fedf8115512420341221 (positive)
          Expectation for test: b537b61c88140e48bb22734d29981ee0 (positive)
          Expectation for test: e626d96fedf8521cd8524c8ebc20b5e5 (positive)
          Expectation for test: 0133f6382f3179df5f87efe0d1d95899 (positive)
          Expectation for test: 5272bb7a944f7b9efb46345a80bc936b (positive)
          Expectation for test: f9501bce78e84a55ce4c8f86d6ea1def (positive)
          Expectation for test: 194b1f256d05dac7284056d4633ee069 (positive)
          Untriaged or negative image:
https://flutter-gold.skia.org/detail?grouping=name%3Dmaterial.input_chip.disabled.delete_button%26source_type%3Dflutter&digest=355e4280d8ca02fbdcf5fe321f0af129


          stderr: Test: material.input_chip.disabled.delete_button FAIL


          result-state.json: No result file found.

When the exception was thrown, this was the stack:
../dart-sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/errors.dart 296:3  throw_
../packages/matcher/src/expect/prints_matcher.dart.js 453:22                    fail
../packages/matcher/src/expect/prints_matcher.dart.js 430:18                    <fn>
../packages/stack_trace/src/stack_zone_specification.dart.js 241:98             <fn>
../packages/stack_trace/src/stack_zone_specification.dart.js 293:16             [_run]
../packages/stack_trace/src/stack_zone_specification.dart.js 241:80             <fn>
../dart-sdk/lib/async/zone.dart 1407:47                                         _rootRunUnary
../dart-sdk/lib/async/zone.dart 1308:19                                         runUnary
../dart-sdk/lib/async/future_impl.dart 163:18                                   handleValue
../dart-sdk/lib/async/future_impl.dart 847:44                                   handleValueCallback
../dart-sdk/lib/async/future_impl.dart 876:13                                   _propagateToListeners
../dart-sdk/lib/async/future_impl.dart 652:5                                    [_completeWithValue]
../dart-sdk/lib/async/future_impl.dart 722:7                                    callback
../dart-sdk/lib/async/schedule_microtask.dart 40:11                             _microtaskLoop
../dart-sdk/lib/async/schedule_microtask.dart 49:5                              _startMicrotaskLoop
../dart-sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart 181:7           <fn>
════════════════════════════════════════════════════════════════════════════════════════════════════

04:56 +914 ~82 -1: test/material/input_chip_test.dart: Delete button is visible on disabled InputChip [E]                                                                                              
  Test failed. See exception logs above.
  The test description was: Delete button is visible on disabled InputChip

@goderbauer goderbauer mentioned this pull request Mar 12, 2024
Merged
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 12, 2024
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 12, 2024
@engine-flutter-autoroll
49f45d2 Reverts "Fail tests on exceptions raised after test completed…
13b4344 
… (#144706)" (flutter/flutter#144970)
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Mar 12, 2024
@engine-flutter-autoroll
Manual roll Flutter from 3bb2e59 to 1ca8873 (11 revisions) (#6304)
d1aeb26 
Manual roll requested by [email protected]

flutter/flutter@3bb2e59...1ca8873

2024-03-12 [email protected] Update integration tests regexes. (flutter/flutter#144847)
2024-03-11 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Fail tests on exceptions raised after test completed (#144706)" (flutter/flutter#144970)
2024-03-11 [email protected] Make TabController communicating creation in constructor. (flutter/flutter#144912)
2024-03-11 [email protected] Fail tests on exceptions raised after test completed (flutter/flutter#144706)
2024-03-11 [email protected] Refactoring `if` chains into `switch` statements (flutter/flutter#144905)
2024-03-11 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Expose build mode in environment of asset transformer processes (#144752)" (flutter/flutter#144957)
2024-03-11 [email protected] Expose build mode in environment of asset transformer processes (flutter/flutter#144752)
2024-03-11 [email protected] Roll Flutter Engine from 9196947bc687 to 6745955bb49e (2 revisions) (flutter/flutter#144946)
2024-03-11 [email protected] Skip test temporarily until headingLevel is added in engine (issue 41â�¦ (flutter/flutter#135077)
2024-03-11 [email protected] Roll Flutter Engine from 3b0b59bb224d to 9196947bc687 (1 revision) (flutter/flutter#144934)
2024-03-11 [email protected] Roll Packages from 0badb43 to d489d84 (3 revisions) (flutter/flutter#144931)

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
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: 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
@gspencergoog gspencergoog mentioned this pull request Mar 12, 2024
Merged
auto-submit bot pushed a commit that referenced this pull request Mar 12, 2024
@goderbauer
Reland "Fail tests on exceptions raised after test completed (#144706)…
77651bc 
…" (#144980)

Reverts #144970

No changes in this PR compared to the original. The test failure was fixed by adding missing awaits in #144978.

Fixes #144353.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 14, 2024
@engine-flutter-autoroll
49f45d2 Reverts "Fail tests on exceptions raised after test completed…
f15765b 
… (#144706)" (flutter/flutter#144970)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fluttergithubbot fluttergithubbot fluttergithubbot approved these changes

@goderbauer goderbauer Awaiting requested review from goderbauer

Assignees

@gspencergoog gspencergoog

Labels

a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. revert of Bot Only: Tracking label for bot. Tracks new revert of pull requests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@gspencergoog @fluttergithubbot