From 489a2cc76e009a7c7a6d4bd3d4f3be1a9db641bd Mon Sep 17 00:00:00 2001 From: Miguel Sousa Date: Thu, 22 Jun 2023 18:31:00 -0700 Subject: [PATCH] Remove `check-` prefix from subcommands of promoted profiles --- .github/workflows/install_run.yml | 19 +++-- Lib/openbakery/cli.py | 57 ++++++-------- Lib/openbakery/commands/check_profile.py | 14 ++-- tests/commands/test_usage.py | 96 +++++++++--------------- 4 files changed, 79 insertions(+), 107 deletions(-) diff --git a/.github/workflows/install_run.yml b/.github/workflows/install_run.yml index 0d28d91..7ebbb3d 100644 --- a/.github/workflows/install_run.yml +++ b/.github/workflows/install_run.yml @@ -45,13 +45,13 @@ jobs: - name: Run Universal checks run: >- - openbakery check-universal + openbakery universal -x win_ascent_and_descent -x os2_metrics_match_hhea -x soft_dotted data/test/source-sans-pro/OTF/SourceSansPro-Regular.otf data/test/source-sans-pro/OTF/SourceSansPro-Italic.otf; - openbakery check-universal + openbakery universal -x win_ascent_and_descent -x os2_metrics_match_hhea -x fsselection @@ -59,9 +59,16 @@ jobs: -x soft_dotted data/test/source-sans-pro/VAR/SourceSansVariable-Roman.ttf + - name: Run Universal check using 'check-profile' + run: >- + openbakery check-profile openbakery.profiles.universal + data/test/mada/Mada-Regular.ttf + --verbose + -c required_tables + - name: Try running UFO Sources checks run: >- - openbakery check-ufo-sources --verbose data/test/test.ufo + openbakery ufo-sources --verbose data/test/test.ufo || exit 0 shell: bash @@ -71,8 +78,8 @@ jobs: - name: Run UFO Sources checks run: >- - openbakery check-ufo-sources --verbose data/test/test.ufo; - openbakery check-ufo-sources -x designspace_has_consistent + openbakery ufo-sources --verbose data/test/test.ufo; + openbakery ufo-sources -x designspace_has_consistent "data/test/stupidfont/Stupid Font.designspace" - name: Install `googlefonts` extra @@ -81,7 +88,7 @@ jobs: - name: Run Google Fonts checks run: >- - openbakery check-googlefonts + openbakery googlefonts -c canonical_filename -c vendor_id -c glyph_coverage diff --git a/Lib/openbakery/cli.py b/Lib/openbakery/cli.py index 003d4fe..966bc07 100644 --- a/Lib/openbakery/cli.py +++ b/Lib/openbakery/cli.py @@ -1,14 +1,12 @@ import argparse -import pkgutil -import runpy import signal import sys from importlib import import_module -import openbakery.commands +from openbakery import __version__ from openbakery.commands.check_profile import main as check_profile_main -CLI_PROFILES = [ +SUBCOMMANDS = [ "adobefonts", "fontbureau", "fontval", @@ -17,14 +15,15 @@ "iso15008", "notofonts", "opentype", - "ufo_sources", + "ufo-sources", "universal", "proposals", + "check-profile", ] def run_profile_check(profilename): - module = import_module(f"openbakery.profiles.{profilename}") + module = import_module(f"openbakery.profiles.{profilename.replace('-', '_')}") sys.exit(check_profile_main(module.profile)) @@ -36,54 +35,42 @@ def signal_handler(sig, frame): def main(): signal.signal(signal.SIGINT, signal_handler) - subcommands = [ - pkg[1] for pkg in pkgutil.walk_packages(openbakery.commands.__path__) - ] + ["check_" + prof for prof in CLI_PROFILES] - - subcommands = [command.replace("_", "-") for command in sorted(subcommands)] - - if len(sys.argv) >= 2 and sys.argv[1] in subcommands: - # Relay to subcommand. + if len(sys.argv) >= 2 and sys.argv[1] in SUBCOMMANDS: subcommand = sys.argv[1] - subcommand_module = subcommand.replace("-", "_") + sys.argv[0] += " " + subcommand del sys.argv[1] # Make this indirection less visible for subcommands. - if ( - subcommand_module.startswith("check_") - and subcommand_module[6:] in CLI_PROFILES - ): - run_profile_check(subcommand_module[6:]) + + if subcommand == "check-profile": + check_profile_main() else: - runpy.run_module( - "openbakery.commands." + subcommand_module, run_name="__main__" - ) + run_profile_check(subcommand) + else: description = ( - "Run openbakery subcommands. Subcommands have their own help " - "messages. These are usually accessible with the -h/--help flag " - "positioned after the subcommand, i.e.: openbakery subcommand -h" + "Run openbakery subcommands. Subcommands have their own help messages;\n" + "to view them add the '-h' (or '--help') option after the subcommand,\n" + "like in this example:\n openbakery universal -h" ) - parser = argparse.ArgumentParser(description=description) + parser = argparse.ArgumentParser( + formatter_class=argparse.RawTextHelpFormatter, description=description + ) parser.add_argument( "subcommand", help="The subcommand to execute", nargs="?", - choices=subcommands, + choices=SUBCOMMANDS, ) parser.add_argument( "--list-subcommands", action="store_true", - help="print the list of subcommands " - "to stdout, separated by a space character. This is " - "usually only used to generate the shell completion code.", - ) - parser.add_argument( - "--version", action="version", version=openbakery.__version__ + help="print list of supported subcommands", ) + parser.add_argument("--version", action="version", version=__version__) args = parser.parse_args() if args.list_subcommands: - print(" ".join(subcommands)) + print("\n".join(SUBCOMMANDS)) else: parser.print_help() diff --git a/Lib/openbakery/commands/check_profile.py b/Lib/openbakery/commands/check_profile.py index e232cee..0e88cd1 100644 --- a/Lib/openbakery/commands/check_profile.py +++ b/Lib/openbakery/commands/check_profile.py @@ -62,7 +62,7 @@ def ArgumentParser(profile, profile_arg=True): if profile_arg: argument_parser.add_argument( - "profile", help="File/Module name," ' must define a openbakery "profile".' + "profile", help="File/Module name, must define an openbakery 'profile'." ) values_keys = profile.setup_argparse(argument_parser) @@ -78,7 +78,7 @@ def ArgumentParser(profile, profile_arg=True): "--checkid", action="append", help=( - "Explicit check-ids (or parts of their name) to be executed. " + "Explicit check-ids (or parts of their name) to be executed.\n" "Use this option multiple times to select multiple checks." ), ) @@ -88,7 +88,7 @@ def ArgumentParser(profile, profile_arg=True): "--exclude-checkid", action="append", help=( - "Exclude check-ids (or parts of their name) from execution. " + "Exclude check-ids (or parts of their name) from execution.\n" "Use this option multiple times to exclude multiple checks." ), ) @@ -106,7 +106,7 @@ def log_levels_get(key): dest="loglevels", const=PASS, action="append_const", - help="Shortcut for `-l PASS`.\n", + help="Shortcut for '-l PASS'.\n", ) argument_parser.add_argument( @@ -135,7 +135,7 @@ def log_levels_get(key): argument_parser.add_argument( "--succinct", action="store_true", - help="This is a slightly more compact and succint" " output layout.", + help="This is a slightly more compact and succint output layout.", ) argument_parser.add_argument( @@ -143,7 +143,7 @@ def log_levels_get(key): "--no-progress", default=False, action="store_true", - help="In a tty as stdout, don't" " render the progress indicators.", + help="Suppress the progress indicators in the console output.", ) argument_parser.add_argument( @@ -151,7 +151,7 @@ def log_levels_get(key): "--no-colors", default=False, action="store_true", - help="No colors for tty output.", + help="Suppress the coloring theme in the console output.", ) argument_parser.add_argument( diff --git a/tests/commands/test_usage.py b/tests/commands/test_usage.py index 6a37379..aedca00 100644 --- a/tests/commands/test_usage.py +++ b/tests/commands/test_usage.py @@ -5,45 +5,49 @@ import pytest +TOOL_NAME = "openbakery" -def test_list_subcommands_has_all_scripts(): - """Tests if the output from running `openbakery --list-subcommands` matches - the openbakery scripts within the bin folder and the promoted profiles.""" - import openbakery.commands - from openbakery.cli import CLI_PROFILES - - commands_dir = os.path.dirname(openbakery.commands.__file__) - - scripts = [ - f.rstrip(".py").replace("_", "-") - for f in os.listdir(commands_dir) - if (f.endswith(".py") and not f.startswith("_")) - ] - scripts = scripts + [("check-" + i).replace("_", "-") for i in CLI_PROFILES] - subcommands = ( - subprocess.check_output(["openbakery", "--list-subcommands"]).decode().split() - ) - assert sorted(scripts) == sorted(subcommands) +def test_list_subcommands_option(capfd): + """Test if 'openbakery --list-subcommands' can run successfully and output + the expected content.""" + from openbakery.cli import SUBCOMMANDS -def test_command_check_googlefonts(): - """Test if `openbakery check-googlefonts` can run successfully`.""" - subprocess.check_output(["openbakery", "check-googlefonts", "-h"]) + subprocess.check_call([TOOL_NAME, "--list-subcommands"]) + captured = capfd.readouterr() + assert captured.out == "\n".join(SUBCOMMANDS) + "\n" - test_font = os.path.join("data", "test", "nunito", "Nunito-Regular.ttf") - subprocess.check_output( +def test_command_check_googlefonts(): + """Test if 'openbakery googlefonts' can run successfully.""" + subprocess.check_call([TOOL_NAME, "googlefonts", "-h"]) + subprocess.check_call( [ - "openbakery", - "check-googlefonts", + TOOL_NAME, + "googlefonts", "-c", "com.google.fonts/check/canonical_filename", - test_font, + os.path.join("data", "test", "nunito", "Nunito-Regular.ttf"), ] ) + with pytest.raises(subprocess.CalledProcessError): + subprocess.check_call([TOOL_NAME, "googlefonts"]) + + +@pytest.mark.parametrize( + "subcommand", + [ + "check-profile", + "opentype", + "ufo-sources", + ], +) +def test_command_check_profile(subcommand): + """Test if 'openbakery ' can run successfully.""" + subprocess.check_call([TOOL_NAME, subcommand, "-h"]) with pytest.raises(subprocess.CalledProcessError): - subprocess.check_output(["openbakery", "check-googlefonts"]) + subprocess.check_call([TOOL_NAME, subcommand]) @pytest.mark.xfail( @@ -54,17 +58,15 @@ def test_command_check_googlefonts(): # Please, only remove the xfail mark once the test is more robust / future proof. def test_status_log_is_indented(): """Test if statuses are printed in a limited boundary.""" - test_font = os.path.join("data", "test", "nunito", "Nunito-Regular.ttf") - result = subprocess.run( [ - "openbakery", - "check-googlefonts", + TOOL_NAME, + "googlefonts", "-c", "old_ttfautohint", "-c", "font_copyright", - test_font, + os.path.join("data", "test", "nunito", "Nunito-Regular.ttf"), ], capture_output=True, ) @@ -92,30 +94,6 @@ def test_status_log_is_indented(): ) -def test_command_check_profile(): - """Test if `openbakery check-profile` can run successfully`.""" - subprocess.check_output(["openbakery", "check-profile", "-h"]) - - with pytest.raises(subprocess.CalledProcessError): - subprocess.check_output(["openbakery", "check-profile"]) - - -def test_command_check_opentype(): - """Test if `openbakery check-opentype` can run successfully`.""" - subprocess.check_output(["openbakery", "check-opentype", "-h"]) - - with pytest.raises(subprocess.CalledProcessError): - subprocess.check_output(["openbakery", "check-opentype"]) - - -def test_command_check_ufo_sources(): - """Test if `openbakery check-ufo-sources` can run successfully`.""" - subprocess.check_output(["openbakery", "check-ufo-sources", "-h"]) - - with pytest.raises(subprocess.CalledProcessError): - subprocess.check_output(["openbakery", "check-ufo-sources"]) - - def test_command_config_file(): """Test if we can set checks using a config file.""" config = tempfile.NamedTemporaryFile(delete=False) @@ -123,7 +101,7 @@ def test_command_config_file(): config.close() test_font = os.path.join("data", "test", "nunito", "Nunito-Regular.ttf") result = subprocess.run( - ["openbakery", "check-googlefonts", "--config", config.name, test_font], + [TOOL_NAME, "googlefonts", "--config", config.name, test_font], stdout=subprocess.PIPE, ) stdout = result.stdout.decode() @@ -145,7 +123,7 @@ def test_command_config_file_injection(): test_profile = os.path.join("tests", "profiles", "a_test_profile.py") result = subprocess.run( [ - "openbakery", + TOOL_NAME, "check-profile", "-C", "--config", @@ -175,7 +153,7 @@ def test_config_override(): config.close() test_font = os.path.join("data", "test", "varfont", "inter", "Inter[slnt,wght].ttf") result = subprocess.run( - ["openbakery", "check-googlefonts", "-C", "--config", config.name, test_font], + [TOOL_NAME, "googlefonts", "-C", "--config", config.name, test_font], stdout=subprocess.PIPE, ) stdout = result.stdout.decode()