timmaffett / material_symbols_icons

Complete Flutter support for google's Material DesignMaterial Symbols Icons
Apache License 2.0
32 stars 5 forks source link

Asset loading issue when testing screens with MUI Symbols via Golden Tests #12

Closed myfatduck-sync closed 4 months ago

myfatduck-sync commented 8 months ago

Hi, I'm running into an issue where I cannot seem to get Golden tests to pass after adding and using this package in a project. I see the material_symbols_icons package with the 3 font files within the build directory, but the asset is having trouble loading during tests. The app runs fine and looks great though.

Unable to load asset: "packages/material_symbols_icons/lib/fonts/MaterialSymbolsOutlined%5BFILL,GRAD,opsz,wght%5D.ttf".
  The asset does not exist or has empty data.
  package:flutter/src/services/asset_bundle.dart 339:7  PlatformAssetBundle.load
  package:golden_toolkit/src/font_loader.dart 30:37     loadAppFonts
  ===== asynchronous gap ===========================
  test/flutter_test_config.dart 25:3                    testExecutable
timmaffett commented 8 months ago

Hi, For golden tests you need to manually load the fonts that are used..
See the 'Including Fonts' section of: https://api.flutter.dev/flutter/flutter_test/matchesGoldenFile.html

for more info. Let me know if this works for you!

-tim

myfatduck-sync commented 8 months ago

Thanks for getting back to me!! If I use this method, I would have to add the Symbols font to the pubspec.yml project as an asset, which would increase the project's bundle size unnecessarily (since this is already added via this package).

Is there a way to import the Symbols font in tests from this package instead?

fredgrott commented 8 months ago

Yes, my understanding is that you have to manually put the raw font files in asset directory and then use loadFonts in the test main setup to load them before the test is executed...so no you will have to duplicate it in assets for now...

derekedelaney commented 4 months ago

I was experiencing this issue as well. I loaded the font manually but it was still happening. After investigating further, I looked at the code at package:golden_toolkit/src/font_loader.dart. This function loads all of the fonts generated in the FontManifest.json file into the tests. Inside that file was this snippet from this package:

{
    "family": "packages/material_symbols_icons/MaterialSymbolsOutlined",
    "fonts": [
      {
        "asset": "packages/material_symbols_icons/lib/fonts/MaterialSymbolsOutlined%5BFILL,GRAD,opsz,wght%5D.ttf"
      }
    ]
  },
  {
    "family": "packages/material_symbols_icons/MaterialSymbolsRounded",
    "fonts": [
      {
        "asset": "packages/material_symbols_icons/lib/fonts/MaterialSymbolsRounded%5BFILL,GRAD,opsz,wght%5D.ttf"
      }
    ]
  },
  {
    "family": "packages/material_symbols_icons/MaterialSymbolsSharp",
    "fonts": [
      {
        "asset": "packages/material_symbols_icons/lib/fonts/MaterialSymbolsSharp%5BFILL,GRAD,opsz,wght%5D.ttf"
      }
    ]
  }

So it looks like the file names are getting encoded and the [] are getting replaced with %5B and %5D. Since the file name does not have the encoded file name its failing. I forked this repo and removed the [] from the file name and I'm no longer seeing this error. Its seems like the solution would be to rename the files to not include [] or encode it.

timmaffett commented 4 months ago

Thanks for investigating @derekedelaney
This is not the first problem caused by the [ and ] in the filenames. I have been using the font names unaltered from how google names them, but I am beginning to think that indeed it would be better to just drop the '[FILL,GRAD,opsz,wght]' from the names. (This is listing the variable axis that the font supports, but the actual font names aren't typically interesting for a flutter developer and the %5B/%5D URL encoding that occurs during the build also causes issues elsewhere). Currently the latest build of official Material Symbols fonts is broken ( https://github.com/google/material-design-icons/issues/1655 ) , but this should be resolved next week when everyone is back to work. I will release a new build at that time and I will add an additional step of renaming the fonts.

timmaffett commented 4 months ago

As of the 4.2716.0 release the font names now automatically have the "[FILL,GRAD,opsz,wght]" removed from their names and will no longer be causing this issue!