Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@GaryQian
Copy link
Contributor

@GaryQian GaryQian commented Oct 26, 2018

As encountered by @jason-simmons, platforms that do not support locale passing currently do not initialize the locale variables in dart:ui Window. This leads to inconsistent null expectation behavior on production environments vs test environments.

This provides a list of minimal default preferred locales to window when the platform in question does not support passing the device locale up to flutter. Test environments can now expect to not obtain null locales after the app is running just as production apps can.

Tests are on the framework-side PR: flutter/flutter#23596

// support passing platform locales to Flutter. This will allow tests on
// these platforms to have similar expected behavior as Android and iOS where
// Locale passing is supported.
if ((_locales == null || _locales.isEmpty) && (Platform.isLinux || Platform.isMacOS || Platform.isFuchsia)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of a hard-coded list of platforms, I recommend using the FLUTTER_TEST environment variable (and only doing it in debug builds). See foundation/platform.dart in the framework repo.

if ((_locales == null || _locales.isEmpty) && (Platform.isLinux || Platform.isMacOS || Platform.isFuchsia)) {
// TODO(garyq): As we add support for passing locales on additional platforms,
// return null for newly supported platforms.
_locales = <Locale>[Locale("en", "US"), Locale("zh", "CN")];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why Chinese?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's probably better to just not have any special case logic here and just rely on the framework to handle the defaulting.

if ((_locales == null || _locales.isEmpty) && (Platform.isLinux || Platform.isMacOS || Platform.isFuchsia)) {
// TODO(garyq): As we add support for passing locales on additional platforms,
// return null for newly supported platforms.
_locales = <Locale>[Locale("en", "US"), Locale("zh", "CN")];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can these default locales be set during startup of flutter_tester?

This would ensure that the default locales aren't used on any platform that isn't a test

@GaryQian
Copy link
Contributor Author

GaryQian commented Oct 26, 2018

As Jason suggested, I think I'll inject the locales in during flutter tester startup. This PR can be closed. Stay tuned for a follow up PR with that.

That way this test-only code can stay separate from the actual code.

@GaryQian GaryQian closed this Oct 26, 2018
@GaryQian
Copy link
Contributor Author

New version: #6689

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants