[com.google.fonts/check/usweightclass] is broken because of expected_font_names condition

yanone commented 1 year ago

The check currently breaks the code tests, so it's inhibiting authoring new checks. One of the test fonts, Mada-Regular.ttf, has a weight class of 428, but the expected_font_names condition returns 428, so checking against it PASSES when it should instead FAIL.

Besides, running the check against the GF library yields zero FAILS but tons of ERRORS:

    ERROR: 456
    FAIL: 0
    WARN: 0
    INFO: 0
    SKIP: 0
    PASS: 2812

with the error looking like this:

    ERROR The condition                                                         
          <FontBakeryCondition:expected_font_names> had an error: KeyError:     
          'fvar'                                                                
          ↳ File                                                                
          "/Users/yanone/.pyenv/versions/3.11.1/Library/Frameworks/Python.framew
          ork/Versions/3.11/lib/python3.11/site-packages/fontbakery/checkrunner.
          py", line 228, in _evaluate_condition                                 
          return None, condition(**args)                                        
          ^^^^^^^^^^^^^^^^^                                                     
          ↳ File                                                                
          "/Users/yanone/.pyenv/versions/3.11.1/Library/Frameworks/Python.framew
          ork/Versions/3.11/lib/python3.11/site-packages/fontbakery/callable.py"
          , line 110, in call                                                   
          return self.__wrapped__(*args, **kwds)                                
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^                                       
          ↳ File                                                                
          "/Users/yanone/.pyenv/versions/3.11.1/Library/Frameworks/Python.framew
          ork/Versions/3.11/lib/python3.11/site-packages/fontbakery/profiles/goo
          glefonts_conditions.py", line 680, in expected_font_names             
          build_stat(font_cp, siblings)                                         
          ↳ File                                                                
          "/Users/yanone/.pyenv/versions/3.11.1/Library/Frameworks/Python.framew
          ork/Versions/3.11/lib/python3.11/site-packages/axisregistry/init.py", 
          line 169, in build_stat                                               
          fallbacks_in_siblings = list(                                         
          ^^^^^                                                                 
          ↳ File                                                                
          "/Users/yanone/.pyenv/versions/3.11.1/Library/Frameworks/Python.framew
          ork/Versions/3.11/lib/python3.11/site-packages/axisregistry/init.py", 
          line 171, in <genexpr>                                                
          axis_registry.fallbacks_in_name_table(f) for f in sibling_ttFonts     
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^                              
          ↳ File                                                                
          "/Users/yanone/.pyenv/versions/3.11.1/Library/Frameworks/Python.framew
          ork/Versions/3.11/lib/python3.11/site-packages/axisregistry/init.py", 
          line 117, in fallbacks_in_name_table                                  
          fvar_axes_in_font = [a.axisTag for a in ttFont["fvar"].axes]          
          ~~~~~~^^^^^^^^                                                        
          ↳ File                                                                
          "/Users/yanone/.pyenv/versions/3.11.1/Library/Frameworks/Python.framew
          ork/Versions/3.11/lib/python3.11/site-packages/fontTools/ttLib/ttFont.
          py", line 442, in getitem                                             
          table = self._readTable(tag)                                          
          ^^^^^^^^^^^^^^^^^^^^                                                  
          ↳ File                                                                
          "/Users/yanone/.pyenv/versions/3.11.1/Library/Frameworks/Python.framew
          ork/Versions/3.11/lib/python3.11/site-packages/fontTools/ttLib/ttFont.
          py", line 449, in _readTable                                          
          data = self.reader[tag]                                               
          ~~~~~~~~~~~^^^^^                                                      
          ↳ File                                                                
          "/Users/yanone/.pyenv/versions/3.11.1/Library/Frameworks/Python.framew
          ork/Versions/3.11/lib/python3.11/site-packages/fontTools/ttLib/sfnt.py
          ", line 108, in getitem                                               
          entry = self.tables[Tag(tag)]                                         
          ~~~~~~~~~~~^^^^^^^^^^

yanone commented 1 year ago

I found this commit which seems to have broken the check: https://github.com/googlefonts/fontbakery/commit/45199dc7a5f30db224f27e4d553b4c12136e4fe8

What I don't understand is why I'm getting the failing code test now and not you guys when you made those changes.

yanone commented 1 year ago

The expected_style condition was removed here: https://github.com/googlefonts/fontbakery/commit/96cb96cba7377b5844470ac40e5666a6aa539df6

yanone commented 1 year ago

The expected_font_names condition has two separate issues (that I can see):

It reads the usweightclass from the font file and returns that, where the old expected_style returned a manually defined weight class. This breaks the logic of the weight class check.
The error message that I posted above is from running the usweightclass check on the entire GF library (fontbakery check-googlefonts -c usweightclass ofl/*/*.ttf) and is most likely related to variable fonts.

Also, it appears like the code tests were ignored after authoring these changes, which inhibits the authoring of new checks. I found this problem by running the code tests on the main fontbakery repo code while authoring another check:

    def test_check_usweightclass():
        """ Checking OS/2 usWeightClass. """
        check = CheckTester(googlefonts_profile,
                            "com.google.fonts/check/usweightclass")

        # Our reference Mada Regular is know to be bad here.
        font = TEST_FILE("mada/Mada-Regular.ttf")
        ttFont = TTFont(font)
>       assert_results_contain(check(ttFont),
                               FAIL, 'bad-value',
                               f'with bad font "{font}" ...')

tests/profiles/googlefonts_test.py:833:

This check passes when it's expected to fail.

fonttools / fontbakery

[com.google.fonts/check/usweightclass] is broken because of expected_font_names condition #4145