Skip to content
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

Add variant to testGoldens #180

Closed
wants to merge 1 commit into from

Conversation

nilsreichardt
Copy link
Contributor

@nilsreichardt nilsreichardt commented Aug 22, 2023

Adds variant to testGoldens to be able to use TestVariant. This can be useful when you have different layout for every TargetPlatform. Can then use TargetPlatformVariant in your test.

Closes #179

@nilsreichardt
Copy link
Contributor Author

@NickalasB Should we append the name of the variant to the file name (my_file_name_android) for TargetPlatform.android? With the current state of the PR it just runs the test for all platforms but uses the same image every time

import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:golden_toolkit/golden_toolkit.dart';
import 'package:sharezone/pages/settings/changelog_page.dart';

void main() {
  group(UpdatePromptCard, () {
    testGoldens(
      'display card as expected',
      (tester) async {
        await tester.pumpWidget(
          MaterialApp(
            home: Scaffold(
              body: Padding(
                padding: const EdgeInsets.all(8.0),
                child: Column(
                  children: [
                    UpdatePromptCard(),
                  ],
                ),
              ),
            ),
          ),
        );

        await screenMatchesGolden(tester, 'update_prompt_card');
      },
      variant: TargetPlatformVariant.all(),
    );
  });
}

@NickalasB
Copy link
Contributor

NickalasB commented Aug 22, 2023

@NickalasB Should we append the name of the variant to the file name (my_file_name_android) for TargetPlatform.android? With the current state of the PR it just runs the test for all platforms but uses the same image every time

import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:golden_toolkit/golden_toolkit.dart';
import 'package:sharezone/pages/settings/changelog_page.dart';

void main() {
  group(UpdatePromptCard, () {
    testGoldens(
      'display card as expected',
      (tester) async {
        await tester.pumpWidget(
          MaterialApp(
            home: Scaffold(
              body: Padding(
                padding: const EdgeInsets.all(8.0),
                child: Column(
                  children: [
                    UpdatePromptCard(),
                  ],
                ),
              ),
            ),
          ),
        );

        await screenMatchesGolden(tester, 'update_prompt_card');
      },
      variant: TargetPlatformVariant.all(),
    );
  });
}

I think in order for this to work you will need to modify the materialAppWrapper platform param.

As I understand it the TargetPlatformVariant sets the debugDefaultTargetPlatformOverride so maybe something like this...

WidgetWrapper materialAppWrapper({
  TargetPlatform? platform,
  Iterable<LocalizationsDelegate<dynamic>>? localizations,
  NavigatorObserver? navigatorObserver,
  Iterable<Locale>? localeOverrides,
  ThemeData? theme,
}) {
  return (child) => MaterialApp(
        localizationsDelegates: localizations,
        supportedLocales: localeOverrides ?? const [Locale('en')],
        theme: theme?.copyWith(
          platform:
              debugDefaultTargetPlatformOverride ?? TargetPlatform.android,
        ),...

@NickalasB
Copy link
Contributor

@nilsreichardt also... for what you are trying to accomplish. Have you tried the multiScreenGolden function?

@nilsreichardt
Copy link
Contributor Author

I have a card that shows different text for each platform (iOS, Android, Web, macOS). I created a golden test for that where I loop over the platforms. My goal with this PR is to make advantage of variant parameter so that I can remove the for loop. However, I'm uncertain if this change is actually good. I would tend to close this PR and suggest as a workaround to use just a for loop.

Here is the code of the test (and the golden files): https://github.com/SharezoneApp/sharezone-app/blob/main/app/test_goldens/pages/settings/changelog_page_test.dart

@NickalasB
Copy link
Contributor

NickalasB commented Aug 28, 2023

@nilsreichardt How about adding a test in your PR to assert that the desired behavior is working as intended, and then we can make the call as to whether it adds value? Thank you very much for helping to contribute!

@nilsreichardt
Copy link
Contributor Author

How about adding a test in your PR to assert that the desired behavior is working as intended, and then we can make the call as to whether it adds value?

The way the PR is currently, does not work because I would need to change the file name for every variant (append the variant name as suffix). I'm going to close it for now.

@NickalasB
Copy link
Contributor

How about adding a test in your PR to assert that the desired behavior is working as intended, and then we can make the call as to whether it adds value?

The way the PR is currently, does not work because I would need to change the file name for every variant (append the variant name as suffix). I'm going to close it for now.

@nilsreichardt No worries. Thank you for supporting and suggesting improvements!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for variant
2 participants