feat: new optional loadFonts parameter

[EArminjon]

Description

fix memory issue along multiple call to loadFonts. https://github.com/Betterment/alchemist/issues/76

As a developer, I just need to put loadFonts: false and call the following code before my multiple goldenTest().

setUpAll(() async {
    await loadFonts();
  });

This fix also increase test speed, it's insane.

Type of Change

New optional parameters

• [x] ✨ New feature (non-breaking change which adds functionality)
• [x] 🛠️ Bug fix (non-breaking change which fixes an issue)
• [ ] ❌ Breaking change (fix or feature that would cause existing functionality to change)
• [ ] 🧹 Code refactor
• [ ] ✅ Build configuration change
• [x] 📝 Documentation
• [ ] 🗑️ Chore

[EArminjon]

Maybe an other solution can be to check inside loadFonts if the fonts already exist. I didn't know if it's possible.

[TesteurManiak]

I find it a bit cumbersome to have to specify goldenTest(loadFonts: false) every time when you might want to disable it globally. Maybe you could consider injecting the loadFonts value with an environment variable 🤔

For example:

flutter test --dart-define=LOAD_GOLDEN_FONTS=false

The implementation in golden_test.dart could be:

const _loadFontsDefault = bool.fromEnvironment('LOAD_GOLDEN_FONTS', defaultValue: true);

// ...

Future<void> goldenTest({
  // ...
  bool? loadFonts,
}) async {
  // ...

  goldenTestAdapter.setUp(
    () => _setUpGoldenTests(mustLoadFonts: loadFonts ?? _loadFontsDefault),
  );
}

[EArminjon]

I find it a bit cumbersome to have to specify goldenTest(loadFonts: false) every time when you might want to disable it globally. Maybe you could consider injecting the loadFonts value with an environment variable 🤔

For example:

flutter test --dart-define=LOAD_GOLDEN_FONTS=false

The implementation in golden_test.dart could be:

const _loadFontsDefault = bool.fromEnvironment('LOAD_GOLDEN_FONTS', defaultValue: true);

// ...

Future<void> goldenTest({
  // ...
  bool? loadFonts,
}) async {
  // ...

  goldenTestAdapter.setUp(
    () => _setUpGoldenTests(mustLoadFonts: loadFonts ?? _loadFontsDefault),
  );
}

I think we can find a better solution. I create this PR as a quick fix because i (and my team) couldn't use this package. Defining env variable is maybe not perfect. If someone forgot it or just run flutter test, he can encounter issues.

[TesteurManiak]

Defining env variable is maybe not perfect. If someone forgot it or just run flutter test, he can encounter issues.

True, maybe adding a property to AlchemistConfig or PlatformGoldensConfig should be the way to go 🤔

[bmt-github-policybot]

This pull request has been automatically closed because it has not been updated in the last 3 months. :sleepy:

If you still need this change, please reopen it and update it within the next 3 months.

Thanks for helping keep our house in order!

[EArminjon]

Don't close please

[EArminjon]

Should I add the parameter in this config ?

[samandmoore]

Sorry about that! We have to make some time to dig into this

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (cd09414) to head (8040f8a). Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #101   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           13        13           
  Lines          562       563    +1     
=========================================
+ Hits           562       563    +1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[bmt-github-policybot]

This pull request has been automatically closed because it has not been updated in the last month. :sleepy:

If you still need this change, you can reopen it.

Thanks for helping keep our house in order!