diff options
| author | Zach White <[email protected]> | 2021-05-10 11:18:44 -0700 | 
|---|---|---|
| committer | GitHub <[email protected]> | 2021-05-10 11:18:44 -0700 | 
| commit | a3e7f3e7c58ee98596ead5c213f3a9ed8340cd80 (patch) | |
| tree | ef0cb85205fad7562045f23caa8a16384e43bbb5 | |
| parent | 66ed80ad3a0edecd9d7abbef71fc2a6e3e59b541 (diff) | |
| download | qmk_firmware-a3e7f3e7c58ee98596ead5c213f3a9ed8340cd80.tar.gz qmk_firmware-a3e7f3e7c58ee98596ead5c213f3a9ed8340cd80.zip | |
Improve our CI tests (#11476)
* add a test and dry-run to qmk generate-api
* add a dry-run to qmk pyformat
* Add a --dry-run to qmk cformat
* reverse the order of nose2 and flake8 tests
* run CI test against cformat and pyformat
* fix programming errors
* tweak job name
* fix argument
* refine the files we select
* fix stack trace in --ci
* make cformat exit clean
* fix c file extensions
* decouple CI from pyformat
* remove --ci arg
* make ci happy
* use the environment var instead
* change output to text
* fix log message
* replace tabs
| -rw-r--r-- | .github/workflows/format.yaml | 57 | ||||
| -rw-r--r-- | lib/python/qmk/cli/cformat.py | 119 | ||||
| -rwxr-xr-x | lib/python/qmk/cli/generate/api.py | 35 | ||||
| -rwxr-xr-x | lib/python/qmk/cli/pyformat.py | 15 | ||||
| -rw-r--r-- | lib/python/qmk/cli/pytest.py | 3 | ||||
| -rw-r--r-- | lib/python/qmk/tests/test_cli_commands.py | 16 | 
6 files changed, 166 insertions, 79 deletions
| diff --git a/.github/workflows/format.yaml b/.github/workflows/format.yaml index 201f3c230f..6d13e78945 100644 --- a/.github/workflows/format.yaml +++ b/.github/workflows/format.yaml @@ -1,47 +1,42 @@ -name: Format Codebase +name: PR Lint Format  on: -  push: -    branches: -    - master -    - develop +  pull_request: +    paths: +    - 'drivers/**' +    - 'lib/arm_atsam/**' +    - 'lib/lib8tion/**' +    - 'lib/python/**' +    - 'platforms/**' +    - 'quantum/**' +    - 'tests/**' +    - 'tmk_core/**'  jobs: -  format: +  lint:      runs-on: ubuntu-latest -    container: qmkfm/base_container -    # protect against those who develop with their fork on master -    if: github.repository == 'qmk/qmk_firmware' +    container: qmkfm/base_container      steps:      - uses: rlespinasse/[email protected]      - uses: actions/checkout@v2        with: -        token: ${{ secrets.API_TOKEN_GITHUB }} - -    - name: Install dependencies -      run: | -        apt-get update && apt-get install -y dos2unix +        fetch-depth: 0 -    - name: Format files -      run: | -        bin/qmk cformat -a -        bin/qmk pyformat -        bin/qmk fileformat +    - uses: trilom/[email protected] +      id: file_changes +      with: +        output: ' ' +        fileOutput: ' ' -    - name: Become QMK Bot +    - name: Run qmk cformat and qmk pyformat +      shell: 'bash {0}'        run: | -        git config user.name 'QMK Bot' -        git config user.email '[email protected]' +        qmk cformat -n $(< ~/files.txt) +        cformat_exit=$? +        qmk pyformat -n +        pyformat_exit=$? -    - name: Create Pull Request -      uses: peter-evans/create-pull-request@v3 -      with: -        delete-branch: true -        branch: bugfix/format_${{ env.GITHUB_REF_SLUG }} -        author: QMK Bot <[email protected]> -        committer: QMK Bot <[email protected]> -        commit-message: Format code according to conventions -        title: '[CI] Format code according to conventions' +        exit $((cformat_exit + pyformat_exit)) diff --git a/lib/python/qmk/cli/cformat.py b/lib/python/qmk/cli/cformat.py index d0d3b3b0a3..9333aaec4b 100644 --- a/lib/python/qmk/cli/cformat.py +++ b/lib/python/qmk/cli/cformat.py @@ -1,6 +1,7 @@  """Format C code according to QMK's style.  """  import subprocess +from os import path  from shutil import which  from argcomplete.completers import FilesCompleter @@ -9,58 +10,118 @@ from milc import cli  from qmk.path import normpath  from qmk.c_parse import c_source_files +c_file_suffixes = ('c', 'h', 'cpp') +core_dirs = ('drivers', 'quantum', 'tests', 'tmk_core', 'platforms') +ignored = ('tmk_core/protocol/usb_hid', 'quantum/template', 'platforms/chibios') -def cformat_run(files, all_files): -    """Spawn clang-format subprocess with proper arguments + +def find_clang_format(): +    """Returns the path to clang-format.      """ -    # Determine which version of clang-format to use -    clang_format = ['clang-format', '-i']      for clang_version in range(20, 6, -1): -        binary = 'clang-format-%d' % clang_version +        binary = f'clang-format-{clang_version}' +          if which(binary): -            clang_format[0] = binary -            break +            return binary + +    return 'clang-format' + + +def find_diffs(files): +    """Run clang-format and diff it against a file. +    """ +    found_diffs = False + +    for file in files: +        cli.log.debug('Checking for changes in %s', file) +        clang_format = subprocess.Popen([find_clang_format(), file], stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True) +        diff = cli.run(['diff', '-u', f'--label=a/{file}', f'--label=b/{file}', str(file), '-'], stdin=clang_format.stdout, capture_output=True) + +        if diff.returncode != 0: +            print(diff.stdout) +            found_diffs = True + +    return found_diffs + + +def cformat_run(files): +    """Spawn clang-format subprocess with proper arguments +    """ +    # Determine which version of clang-format to use +    clang_format = [find_clang_format(), '-i'] +      try: -        if not files: -            cli.log.warn('No changes detected. Use "qmk cformat -a" to format all files') -            return False -        subprocess.run(clang_format + [file for file in files], check=True) +        cli.run(clang_format + list(map(str, files)), check=True, capture_output=False)          cli.log.info('Successfully formatted the C code.') +        return True -    except subprocess.CalledProcessError: +    except subprocess.CalledProcessError as e:          cli.log.error('Error formatting C code!') +        cli.log.debug('%s exited with returncode %s', e.cmd, e.returncode) +        cli.log.debug('STDOUT:') +        cli.log.debug(e.stdout) +        cli.log.debug('STDERR:') +        cli.log.debug(e.stderr)          return False [email protected]('-a', '--all-files', arg_only=True, action='store_true', help='Format all core files.') +def filter_files(files): +    """Yield only files to be formatted and skip the rest +    """ +    for file in files: +        if file.name.split('.')[-1] in c_file_suffixes: +            yield file +        else: +            cli.log.debug('Skipping file %s', file) + + [email protected]('-n', '--dry-run', arg_only=True, action='store_true', help="Flag only, don't automatically format.")  @cli.argument('-b', '--base-branch', default='origin/master', help='Branch to compare to diffs to.') [email protected]('files', nargs='*', arg_only=True, completer=FilesCompleter('.c'), help='Filename(s) to format.') [email protected]('-a', '--all-files', arg_only=True, action='store_true', help='Format all core files.') [email protected]('files', nargs='*', arg_only=True, type=normpath, completer=FilesCompleter('.c'), help='Filename(s) to format.')  @cli.subcommand("Format C code according to QMK's style.", hidden=False if cli.config.user.developer else True)  def cformat(cli):      """Format C code according to QMK's style.      """ -    # Empty array for files -    files = [] -    # Core directories for formatting -    core_dirs = ['drivers', 'quantum', 'tests', 'tmk_core', 'platforms'] -    ignores = ['tmk_core/protocol/usb_hid', 'quantum/template', 'platforms/chibios']      # Find the list of files to format      if cli.args.files: -        files.extend(normpath(file) for file in cli.args.files) +        files = list(filter_files(cli.args.files)) + +        if not files: +            cli.log.error('No C files in filelist: %s', ', '.join(map(str, cli.args.files))) +            exit(0) +          if cli.args.all_files:              cli.log.warning('Filenames passed with -a, only formatting: %s', ','.join(map(str, files))) -    # If -a is specified +      elif cli.args.all_files:          all_files = c_source_files(core_dirs)          # The following statement checks each file to see if the file path is in the ignored directories. -        files.extend(file for file in all_files if not any(i in str(file) for i in ignores)) -    # No files specified & no -a flag +        files = [file for file in all_files if not any(i in str(file) for i in ignored)] +      else: -        base_args = ['git', 'diff', '--name-only', cli.args.base_branch] -        out = subprocess.run(base_args + core_dirs, check=True, stdout=subprocess.PIPE) -        changed_files = filter(None, out.stdout.decode('UTF-8').split('\n')) -        filtered_files = [normpath(file) for file in changed_files if not any(i in file for i in ignores)] -        files.extend(file for file in filtered_files if file.exists() and file.suffix in ['.c', '.h', '.cpp']) +        git_diff_cmd = ['git', 'diff', '--name-only', cli.args.base_branch, *core_dirs] +        git_diff = cli.run(git_diff_cmd) + +        if git_diff.returncode != 0: +            cli.log.error("Error running %s", git_diff_cmd) +            print(git_diff.stderr) +            return git_diff.returncode + +        files = [] + +        for file in git_diff.stdout.strip().split('\n'): +            if not any([file.startswith(ignore) for ignore in ignored]): +                if path.exists(file) and file.split('.')[-1] in c_file_suffixes: +                    files.append(file) + +    # Sanity check +    if not files: +        cli.log.error('No changed files detected. Use "qmk cformat -a" to format all files') +        return False      # Run clang-format on the files we've found -    cformat_run(files, cli.args.all_files) +    if cli.args.dry_run: +        return not find_diffs(files) +    else: +        return cformat_run(files) diff --git a/lib/python/qmk/cli/generate/api.py b/lib/python/qmk/cli/generate/api.py index 70019428f0..8ab7522a76 100755 --- a/lib/python/qmk/cli/generate/api.py +++ b/lib/python/qmk/cli/generate/api.py @@ -13,6 +13,7 @@ from qmk.json_schema import json_load  from qmk.keyboard import list_keyboards [email protected]('-n', '--dry-run', arg_only=True, action='store_true', help="Don't write the data to disk.")  @cli.subcommand('Creates a new keymap for the keyboard of your choosing', hidden=False if cli.config.user.developer else True)  def generate_api(cli):      """Generates the QMK API data. @@ -40,10 +41,14 @@ def generate_api(cli):          keyboard_readme_src = Path('keyboards') / keyboard_name / 'readme.md'          keyboard_dir.mkdir(parents=True, exist_ok=True) -        keyboard_info.write_text(json.dumps({'last_updated': current_datetime(), 'keyboards': {keyboard_name: kb_all[keyboard_name]}})) +        keyboard_json = json.dumps({'last_updated': current_datetime(), 'keyboards': {keyboard_name: kb_all[keyboard_name]}}) +        if not cli.args.dry_run: +            keyboard_info.write_text(keyboard_json) +            cli.log.debug('Wrote file %s', keyboard_info) -        if keyboard_readme_src.exists(): -            copyfile(keyboard_readme_src, keyboard_readme) +            if keyboard_readme_src.exists(): +                copyfile(keyboard_readme_src, keyboard_readme) +                cli.log.debug('Copied %s -> %s', keyboard_readme_src, keyboard_readme)          if 'usb' in kb_all[keyboard_name]:              usb = kb_all[keyboard_name]['usb'] @@ -57,20 +62,26 @@ def generate_api(cli):              if 'vid' in usb and 'pid' in usb:                  usb_list[usb['vid']][usb['pid']][keyboard_name] = usb -    # Write the global JSON files -    keyboard_all_file.write_text(json.dumps({'last_updated': current_datetime(), 'keyboards': kb_all}, cls=InfoJSONEncoder)) -    usb_file.write_text(json.dumps({'last_updated': current_datetime(), 'usb': usb_list}, cls=InfoJSONEncoder)) - +    # Generate data for the global files      keyboard_list = sorted(kb_all) -    keyboard_list_file.write_text(json.dumps({'last_updated': current_datetime(), 'keyboards': keyboard_list}, cls=InfoJSONEncoder)) -      keyboard_aliases = json_load(Path('data/mappings/keyboard_aliases.json')) -    keyboard_aliases_file.write_text(json.dumps({'last_updated': current_datetime(), 'keyboard_aliases': keyboard_aliases}, cls=InfoJSONEncoder)) -      keyboard_metadata = {          'last_updated': current_datetime(),          'keyboards': keyboard_list,          'keyboard_aliases': keyboard_aliases,          'usb': usb_list,      } -    keyboard_metadata_file.write_text(json.dumps(keyboard_metadata, cls=InfoJSONEncoder)) + +    # Write the global JSON files +    keyboard_all_json = json.dumps({'last_updated': current_datetime(), 'keyboards': kb_all}, cls=InfoJSONEncoder) +    usb_json = json.dumps({'last_updated': current_datetime(), 'usb': usb_list}, cls=InfoJSONEncoder) +    keyboard_list_json = json.dumps({'last_updated': current_datetime(), 'keyboards': keyboard_list}, cls=InfoJSONEncoder) +    keyboard_aliases_json = json.dumps({'last_updated': current_datetime(), 'keyboard_aliases': keyboard_aliases}, cls=InfoJSONEncoder) +    keyboard_metadata_json = json.dumps(keyboard_metadata, cls=InfoJSONEncoder) + +    if not cli.args.dry_run: +        keyboard_all_file.write_text(keyboard_all_json) +        usb_file.write_text(usb_json) +        keyboard_list_file.write_text(keyboard_list_json) +        keyboard_aliases_file.write_text(keyboard_aliases_json) +        keyboard_metadata_file.write_text(keyboard_metadata_json) diff --git a/lib/python/qmk/cli/pyformat.py b/lib/python/qmk/cli/pyformat.py index 1464443804..02581f0d85 100755 --- a/lib/python/qmk/cli/pyformat.py +++ b/lib/python/qmk/cli/pyformat.py @@ -5,13 +5,22 @@ from milc import cli  import subprocess [email protected]('-n', '--dry-run', arg_only=True, action='store_true', help="Flag only, don't automatically format.")  @cli.subcommand("Format python code according to QMK's style.", hidden=False if cli.config.user.developer else True)  def pyformat(cli):      """Format python code according to QMK's style.      """ +    edit = '--diff' if cli.args.dry_run else '--in-place' +    yapf_cmd = ['yapf', '-vv', '--recursive', edit, 'bin/qmk', 'lib/python']      try: -        subprocess.run(['yapf', '-vv', '-ri', 'bin/qmk', 'lib/python'], check=True) -        cli.log.info('Successfully formatted the python code in `bin/qmk` and `lib/python`.') +        cli.run(yapf_cmd, check=True, capture_output=False) +        cli.log.info('Python code in `bin/qmk` and `lib/python` is correctly formatted.') +        return True      except subprocess.CalledProcessError: -        cli.log.error('Error formatting python code!') +        if cli.args.dry_run: +            cli.log.error('Python code in `bin/qmk` and `lib/python` incorrectly formatted!') +        else: +            cli.log.error('Error formatting python code!') + +    return False diff --git a/lib/python/qmk/cli/pytest.py b/lib/python/qmk/cli/pytest.py index 5417a9cb34..50a1d70a4f 100644 --- a/lib/python/qmk/cli/pytest.py +++ b/lib/python/qmk/cli/pytest.py @@ -11,6 +11,7 @@ from milc import cli  def pytest(cli):      """Run several linting/testing commands.      """ -    flake8 = subprocess.run(['flake8', 'lib/python', 'bin/qmk'])      nose2 = subprocess.run(['nose2', '-v']) +    flake8 = subprocess.run(['flake8', 'lib/python', 'bin/qmk']) +      return flake8.returncode | nose2.returncode diff --git a/lib/python/qmk/tests/test_cli_commands.py b/lib/python/qmk/tests/test_cli_commands.py index c57d2b7fc7..741551e5ec 100644 --- a/lib/python/qmk/tests/test_cli_commands.py +++ b/lib/python/qmk/tests/test_cli_commands.py @@ -33,10 +33,15 @@ def check_returncode(result, expected=[0]):  def test_cformat(): -    result = check_subcommand('cformat', 'quantum/matrix.c') +    result = check_subcommand('cformat', '-n', 'quantum/matrix.c')      check_returncode(result) +def test_cformat_all(): +    result = check_subcommand('cformat', '-n', '-a') +    check_returncode(result, [0, 1]) + +  def test_compile():      result = check_subcommand('compile', '-kb', 'handwired/pytest/basic', '-km', 'default', '-n')      check_returncode(result) @@ -83,9 +88,9 @@ def test_hello():  def test_pyformat(): -    result = check_subcommand('pyformat') +    result = check_subcommand('pyformat', '--dry-run')      check_returncode(result) -    assert 'Successfully formatted the python code' in result.stdout +    assert 'Python code in `bin/qmk` and `lib/python` is correctly formatted.' in result.stdout  def test_list_keyboards(): @@ -225,6 +230,11 @@ def test_clean():      assert result.stdout.count('done') == 2 +def test_generate_api(): +    result = check_subcommand('generate-api', '--dry-run') +    check_returncode(result) + +  def test_generate_rgb_breathe_table():      result = check_subcommand("generate-rgb-breathe-table", "-c", "1.2", "-m", "127")      check_returncode(result) | 
