nit: Is this toString override needed? I think when trying this out the main difference I saw is when all SystemUiOverlayStyle has all null properties it will print out something like SystemUiOverlayStyle#12345, but with the additional toString override we get SystemUiOverlayStyle({systemNavigationBarColor: null, systemNavigationBarDividerColor: null, systemStatusBarContrastEnforced: null, statusBarColor: null, statusBarBrightness: null, statusBarIconBrightness: null, systemNavigationBarIconBrightness: null, systemNavigationBarContrastEnforced: null}). I think maybe we can accomplish the same thing by leaving omitting the defaultValue from the DiagnosticsPropertys.

Thanks for reviewing this!

If the toString method is not overridden, the existing test will fail:

Expected: 'SystemUiOverlayStyle({systemNavigationBarColor: null, systemNavigationBarDividerColor: null, systemStatusBarContrastEnforced: null, statusBarColor: null, statusBarBrightness: null, statusBarIconBrightness: null, systemNavigationBarIconBrightness: null, systemNavigationBarContrastEnforced: null})'
  Actual: 'SystemUiOverlayStyle#ae764'
   Which: is different.

This test is an existing test named toString works as intended. For the color properties, I believe it is more meaningful to verify the actual color values rather than the hash code.

I definitely agree its more valuable to see the color values then the hash code. I tried the following:

  @override
  void debugFillProperties(DiagnosticPropertiesBuilder properties) {
    super.debugFillProperties(properties);
    properties.add(
      DiagnosticsProperty<Color>(
        'systemNavigationBarColor',
        systemNavigationBarColor,
      ),
    );
    properties.add(
      DiagnosticsProperty<Color>(
        'systemNavigationBarDividerColor',
        systemNavigationBarDividerColor,
      ),
    );
    properties.add(
      DiagnosticsProperty<Brightness>(
        'systemNavigationBarIconBrightness',
        systemNavigationBarIconBrightness,
      ),
    );
    properties.add(
      DiagnosticsProperty<bool>(
        'systemNavigationBarContrastEnforced',
        systemNavigationBarContrastEnforced,
      ),
    );
    properties.add(
      DiagnosticsProperty<Color>(
        'statusBarColor',
        statusBarColor,
      ),
    );
    properties.add(
      DiagnosticsProperty<Brightness>(
        'statusBarBrightness',
        statusBarBrightness,
      ),
    );
    properties.add(
      DiagnosticsProperty<Brightness>(
        'statusBarIconBrightness',
        statusBarIconBrightness,
      ),
    );
    properties.add(
      DiagnosticsProperty<bool>(
        'systemStatusBarContrastEnforced',
        systemStatusBarContrastEnforced,
      ),
    );
  }

without the toString override and got the following result:

SystemUiOverlayStyle#8e307(systemNavigationBarColor: null, systemNavigationBarDividerColor: null, systemNavigationBarIconBrightness: null, systemNavigationBarContrastEnforced: null, statusBarColor: null, statusBarBrightness: null, statusBarIconBrightness: null, systemStatusBarContrastEnforced: null)

Is this the same as with the toString override?

Wew, looks like we are seeing the different outputs here. If I don't override toString, it's only SystemUiOverlayStyle#4f905 on my end.

I also removed the defaultValue from the DiagnosticsProperty. If we set defaultValue to null then the defaultValue will match the actual value (null in the context of the existing test) of the property and won't be included in the final string. To avoid this we should omit defaultValue from our DiagnosticsPropertys.

00:03 +8 -1: toString works as intended [E]                                                                                                                                                      
  Expected: 'SystemUiOverlayStyle({systemNavigationBarColor: null, systemNavigationBarDividerColor: null, systemStatusBarContrastEnforced: null, statusBarColor: null, statusBarBrightness: null, statusBarIconBrightness: null, systemNavigationBarIconBrightness: null, systemNavigationBarContrastEnforced: null})'
    Actual: 'SystemUiOverlayStyle#b5187(systemNavigationBarColor: null, systemNavigationBarDividerColor: null, systemNavigationBarIconBrightness: null, systemNavigationBarContrastEnforced: null, statusBarColor: null, statusBarBrightness: null, statusBarIconBrightness: null, systemStatusBarContrastEnforced: null)'
     Which: is different.
            Expected: ... erlayStyle({systemNa ...
              Actual: ... erlayStyle#b5187(sys ...
                                    ^
             Differ at offset 20
  
  package:matcher                                     expect
  package:flutter_test/src/widget_tester.dart 473:18  expect
  test/services/system_chrome_test.dart 231:5         main.<fn>

The existing test still fails but provides the same amount of information. I think unless the formatting is breaking customer tests this should be fine to adjust.

If I include defaultValue: null in the DiagnosticsProperty then I see the output you are seeing for the existing test.

00:03 +8 -1: toString works as intended [E]                                                                                                                                                      
  Expected: 'SystemUiOverlayStyle({systemNavigationBarColor: null, systemNavigationBarDividerColor: null, systemStatusBarContrastEnforced: null, statusBarColor: null, statusBarBrightness: null, statusBarIconBrightness: null, systemNavigationBarIconBrightness: null, systemNavigationBarContrastEnforced: null})'
    Actual: 'SystemUiOverlayStyle#d32d5'
     Which: is different.
            Expected: ... erlayStyle({systemNa ...
              Actual: ... erlayStyle#d32d5
                                    ^
             Differ at offset 20
  
  package:matcher                                     expect
  package:flutter_test/src/widget_tester.dart 473:18  expect
  test/services/system_chrome_test.dart 231:5         main.<fn>

Thanks for the details. If I remove defaultValue: null from DiagnosticsProperty(s), and not overriding toString, I'm seeing the similar result as yours now (the existing test is still failed):

Expected: 'SystemUiOverlayStyle({systemNavigationBarColor: null, systemNavigationBarDividerColor: null, systemStatusBarContrastEnforced: null, statusBarColor: null, statusBarBrightness: null, statusBarIconBrightness: null, systemNavigationBarIconBrightness: null, systemNavigationBarContrastEnforced: null})'
  Actual: 'SystemUiOverlayStyle#900df(systemNavigationBarColor: null, systemNavigationBarDividerColor: null, systemNavigationBarIconBrightness: null, systemNavigationBarContrastEnforced: null, statusBarColor: null, statusBarBrightness: null, statusBarIconBrightness: null, systemStatusBarContrastEnforced: null)'
   Which: is different.
          Expected: ... erlayStyle({systemNa ...
            Actual: ... erlayStyle#900df(sys ...
                                  ^
           Differ at offset 20

SystemUiOverlayStyle#900df is Diagnosticable.toString()'s default implementation, I think. Thus, overriding toString is still necessary here. Does this make sense?

If you're ok with this, I will remove defaultValue: null from DiagnosticsProperty(s), but keep overriding toString.

That makes sense but I think it would be okay to change the expected output of the existing test to match the new output that we get without the defaultValue: null or toString override as long as customer tests and google tests are passing.