Betterment / alchemist

A Flutter tool that makes golden testing easy.
MIT License
268 stars 36 forks source link

Flutter 3.16 is breaking goldens background color #103

Closed TesteurManiak closed 2 months ago

TesteurManiak commented 1 year ago

Is there an existing issue for this?

Version

0.7.0

Description

After upgrading from Flutter 3.13.8 to 3.16.0 my golden test is breaking because of the background color.

Steps to reproduce

Minimal reproducible example

main.dart

void main() {
  runApp(const MainApp());
}

class MainApp extends StatelessWidget {
  const MainApp({super.key});

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      theme: ThemeData(useMaterial3: false),
      home: const HomePage(),
    );
  }
}

class HomePage extends StatelessWidget {
  const HomePage({super.key});

  @override
  Widget build(BuildContext context) {
    return const Scaffold(
      body: Center(
        child: Text('Hello World!'),
      ),
    );
  }
}

home_golden_test.dart

const iphoneSE = Size(320, 568);

void main() {
  goldenTest(
    'HomePage',
    fileName: 'home_page',
    builder: () {
      return GoldenTestGroup(children: [
        GoldenTestScenario(
          constraints: BoxConstraints.tight(iphoneSE),
          name: 'default',
          child: Theme(
            data: ThemeData(useMaterial3: false),
            child: const HomePage(),
          ),
        ),
      ]);
    },
  );
}

Steps to reproduce

1) Run flutter test --update-goldens with Flutter 3.13.8 2) Switch to Flutter 3.16.0 3) Run flutter test -t golden

Expected behavior

The test should not fail.

Screenshots

3.13.8 3.16.0
home_page home_page_2

Additional context and comments

I must admit that I never understood why the background color was based off the theme data considering that it could break after any bump of the SDK like here 🤔

Bassiuz commented 11 months ago

Hello @TesteurManiak,

I encountered the same problem; and figured out it had to do with the forced Material3 used in Flutter themes starting from 3.16. You can use a theme in the widgets you render, but you can also provide a theme to the Alchemist configuration itself to render widgets in that theme.

If you use this in your flutter_test_config.dart it should render in the old background color

return AlchemistConfig.runWithConfig(
    config: AlchemistConfig(
      theme: ThemeData(useMaterial3: false),
      ),
    ),
    run: testMain,
  );
krispypen commented 10 months ago

I prefer the old way, so with the blue background. When it has a background color which is not white or black it's easier to spot transparent issues (for example rounded corners which should be transparent), and it's easier to see the "size" of the widget

vanlooverenkoen commented 10 months ago

I don't think we should be relying on material but have a custom AlchemistTheme where we can set

And stay away from anything related to material. Especially when you want to have consistently generated golden files. You don't want to be bother by material that is used by the testing framework you are using.

vanlooverenkoen commented 2 months ago

@btrautmann it would be nice to have a custom AlchemistTheme so we don't need to rely on material and changes to material itself.

btrautmann commented 2 months ago

@btrautmann it would be nice to have a custom AlchemistTheme so we don't need to rely on material and changes to material itself.

Agreed that we should be material-agnostic. My current plan is as follows (feel free to let me know what you think):

vanlooverenkoen commented 2 months ago

@btrautmann I think that is a good idea!! Implementing a custom theme is basicly just creating a custom object that is provided with the config.

In our case we wrap the config & setup of the test in a seperate function to simplify migration & breaking changes.

btrautmann commented 2 months ago

Update: We recently released 0.8.0 which has a minimum version of 3.16.0. As part of that (and as mentioned above), we re-generated the golden images to be compatible with the latest Flutter changes. The goal is to do that as seldomly as necessary in the future, but here it was a necessity. I've opened https://github.com/Betterment/alchemist/pull/123 as a way of getting more advanced notice of these types of breaks in the future, but will need to find a way to decouple alchemist from the Material changes that keep having downstream impacts before we can enable that workflow, as mentioned in https://github.com/Betterment/alchemist/issues/120.

I'm going to close this issue in the meantime.

btrautmann commented 2 months ago

@btrautmann it would be nice to have a custom AlchemistTheme so we don't need to rely on material and changes to material itself.

@vanlooverenkoen you can see the theme added in https://github.com/Betterment/alchemist/releases/tag/v0.9.0. Let me know if it suits your needs 👍