Regression: Const constructors don't work for widgets on master

[rrousselGit]

The title says it all:
On master, const constructors simply do not work anymore for widgets.

Consider the following snippet:

// test/my_test.dart
import 'package:flutter/cupertino.dart';

void main() {
  print(identical(const SizedBox(), const SizedBox()));
  print(identical(const A(), const A()));
}

class A  {
  const A();
}

executed using flutter test test/my_test.dart

This will print:

false
true

when the expected result is:

true
true

This issue happens on the master branch, but not on stable.

Flutter doctor:

Doctor summary (to see all details, run flutter doctor -v):
[✓] Flutter (Channel unknown, v1.11.1-pre.39, on Mac OS X 10.14.5 18F132, locale en-FR)
 
[!] Android toolchain - develop for Android devices (Android SDK version 28.0.3)
    ! Some Android licenses not accepted.  To resolve this, run: flutter doctor --android-licenses
[!] Xcode - develop for iOS and macOS (Xcode 11.2.1)
    ✗ CocoaPods not installed.
        CocoaPods is used to retrieve the iOS and macOS platform side's plugin code that responds to your plugin usage on the Dart side.
        Without CocoaPods, plugins will not work on iOS or macOS.
        For more info, see https://flutter.dev/platform-plugins
      To install:
        sudo gem install cocoapods
[✓] Android Studio (version 3.1)
[✓] VS Code (version 1.39.2)
[!] Connected device
    ! No devices available

! Doctor found issues in 3 categories.

[zanderso]

@HansMuller @a-siva

[jonahwilliams]

@jacob314

This only happens when track-widget-creation is enabled

[jacob314]

This has always been a side effect of the --track-widget-creation.
There isn't a way around it unless we modified the implementation of identical to lie which could have other unintended side effects.

You still get the const canonicalization that matters most for widgets just not full const canonicalization.
For example:

void main() {
  Widget buildWidget() {
    return const SizedBox()
  }
  var w1 = buildWidget();
  var w2 = buildWidget();
  print(identical(w1, w2));
}

will print true.

[jonahwilliams]

This is only a debug mode feature so it wont effect running applications (and it was previously the default for IDEs).

It sounds like it has some confusing behavior for tests though, so perhaps we should turn it off there?

[rrousselGit]

It's not just confusing, it's a behavior change, which can generate false negative or false positives.

I noticed this issue because I have a test that looks like:

await tester.pump(
  MyInheritedWidget(
    value: 0,
    child: const MyConsumer(),
  ),
);

expect(find.text('0'), findsOneWidget);

await tester.pump(
  MyInheritedWidget(
    value: 1,
    child: const MyConsumer(),
  ),
);

expect(find.text('1'), findsOneWidget);

And the test failed on stable but succeeded on master because of this issue.

[jacob314]

I would instead encourage people to fix any tests that fail with --track-widget-creation and pass without it. Having --track-widget-creation on for tests will let us do a better job showing errors reported as part of tests.
Something seems a bit odd about your code if you can't find the text '1' in that case but you could with --track-widget-creation off.
My isn't the slightly different MyConsumer widget getting the updated inherited value?

Btw to work around the error you can write:

const consumer = MyConsumer();
await tester.pump(
  MyInheritedWidget(
    value: 0,
    child: consumer,
  ),
);

expect(find.text('0'), findsOneWidget);

await tester.pump(
  MyInheritedWidget(
    value: 1,
    child: consumer,
  ),
);

expect(find.text('1'), findsOneWidget);

[rrousselGit]

Of course, this can be worked around by caching the instance manually and not using const.
But that's not the point.

This issue means that const behaves differently in debug vs release mode, which is heavily unexpected.

If I didn't have a CI that runs tests on both stable and master, I would have missed that fact and deployed the code in production with a critical bug.

Also, I wouldn't expect people to naturally write the workaround when they can use the const variant.
It's unlikely that people will find this issue naturally, and therefore likely that they'll have an unexpected weakness in their tests.

[rrousselGit]

Outside of tests, this behavior will also confuse debug tools like:

• Android Studio's spinner that show which widgets are rebuilding
• devtools' timeline

[jacob314]

perhaps the best practice is to generally run tests in both debug and release mode to ensure that code will work in release and debug mode.
You could also easily write code that depends on some other debug only functionality in Flutter. For example, all getters in Flutter that start with the word debug will behave differently in debug and release mode.

[jacob314]

Based on the very small number of tests in package:flutter that had to be tweaked to work with both --track-widget-creation and without it, I expect this may not actually be a large issue in practice.
Had you previously exclusively run your tests from the command line rather than from an IDE?