[PATCH v2 1/8] patman: cosmetic: Fix PEP 8 warnings for the gitutil module.

[PATCH v2 1/8] patman: cosmetic: Fix PEP 8 warnings for the gitutil module.

[Maxim Cournoyer]

This patch fixes all the PEP 8 warnings reported by Pyflake for the
gitutil module.

Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
---

(no changes since v1)

 tools/patman/gitutil.py | 106 ++++++++++++++++++++++++----------------
 1 file changed, 65 insertions(+), 41 deletions(-)

diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py
index ceaf2ce150..74c6e94494 100644
--- a/tools/patman/gitutil.py
+++ b/tools/patman/gitutil.py
@@ -2,21 +2,19 @@
 # Copyright (c) 2011 The Chromium OS Authors.
 #
 
-import re
 import os
-import subprocess
 import sys
 
 from patman import command
 from patman import settings
 from patman import terminal
-from patman import tools
 
 # True to use --no-decorate - we check this in setup()
 use_no_decorate = True
 
+
 def log_cmd(commit_range, git_dir=None, oneline=False, reverse=False,
-           count=None):
+            count=None):
     """Create a command to perform a 'git log'
 
     Args:
@@ -49,6 +47,7 @@ def log_cmd(commit_range, git_dir=None, oneline=False, reverse=False,
     cmd.append('--')
     return cmd
 
+
 def count_commits_to_branch(branch):
     """Returns number of commits between HEAD and the tracking branch.
 
@@ -68,13 +67,14 @@ def count_commits_to_branch(branch):
         rev_range = '@{upstream}..'
     pipe = [log_cmd(rev_range, oneline=True)]
     result = command.run_pipe(pipe, capture=True, capture_stderr=True,
-                             oneline=True, raise_on_error=False)
+                              oneline=True, raise_on_error=False)
     if result.return_code:
         raise ValueError('Failed to determine upstream: %s' %
                          result.stderr.strip())
     patch_count = len(result.stdout.splitlines())
     return patch_count
 
+
 def name_revision(commit_hash):
     """Gets the revision name for a commit
 
@@ -91,6 +91,7 @@ def name_revision(commit_hash):
     name = stdout.split(' ')[1].strip()
     return name
 
+
 def guess_upstream(git_dir, branch):
     """Tries to guess the upstream for a branch
 
@@ -109,7 +110,7 @@ def guess_upstream(git_dir, branch):
     """
     pipe = [log_cmd(branch, git_dir=git_dir, oneline=True, count=100)]
     result = command.run_pipe(pipe, capture=True, capture_stderr=True,
-                             raise_on_error=False)
+                              raise_on_error=False)
     if result.return_code:
         return None, "Branch '%s' not found" % branch
     for line in result.stdout.splitlines()[1:]:
@@ -121,6 +122,7 @@ def guess_upstream(git_dir, branch):
             return name, "Guessing upstream as '%s'" % name
     return None, "Cannot find a suitable upstream for branch '%s'" % branch
 
+
 def get_upstream(git_dir, branch):
     """Returns the name of the upstream for a branch
 
@@ -135,10 +137,10 @@ def get_upstream(git_dir, branch):
     """
     try:
         remote = command.output_one_line('git', '--git-dir', git_dir, 'config',
-                                       'branch.%s.remote' % branch)
+                                         'branch.%s.remote' % branch)
         merge = command.output_one_line('git', '--git-dir', git_dir, 'config',
-                                      'branch.%s.merge' % branch)
-    except:
+                                        'branch.%s.merge' % branch)
+    except Exception:
         upstream, msg = guess_upstream(git_dir, branch)
         return upstream, msg
 
@@ -149,7 +151,8 @@ def get_upstream(git_dir, branch):
         return '%s/%s' % (remote, leaf), None
     else:
         raise ValueError("Cannot determine upstream branch for branch "
-                "'%s' remote='%s', merge='%s'" % (branch, remote, merge))
+                         "'%s' remote='%s', merge='%s'"
+                         % (branch, remote, merge))
 
 
 def get_range_in_branch(git_dir, branch, include_upstream=False):
@@ -168,6 +171,7 @@ def get_range_in_branch(git_dir, branch, include_upstream=False):
     rstr = '%s%s..%s' % (upstream, '~' if include_upstream else '', branch)
     return rstr, msg
 
+
 def count_commits_in_range(git_dir, range_expr):
     """Returns the number of commits in the given range.
 
@@ -180,12 +184,13 @@ def count_commits_in_range(git_dir, range_expr):
     """
     pipe = [log_cmd(range_expr, git_dir=git_dir, oneline=True)]
     result = command.run_pipe(pipe, capture=True, capture_stderr=True,
-                             raise_on_error=False)
+                              raise_on_error=False)
     if result.return_code:
         return None, "Range '%s' not found or is invalid" % range_expr
     patch_count = len(result.stdout.splitlines())
     return patch_count, None
 
+
 def count_commits_in_branch(git_dir, branch, include_upstream=False):
     """Returns the number of commits in the given branch.
 
@@ -201,6 +206,7 @@ def count_commits_in_branch(git_dir, branch, include_upstream=False):
         return None, msg
     return count_commits_in_range(git_dir, range_expr)
 
+
 def count_commits(commit_range):
     """Returns the number of commits in the given range.
 
@@ -215,6 +221,7 @@ def count_commits(commit_range):
     patch_count = int(stdout)
     return patch_count
 
+
 def checkout(commit_hash, git_dir=None, work_tree=None, force=False):
     """Checkout the selected commit for this build
 
@@ -231,10 +238,11 @@ def checkout(commit_hash, git_dir=None, work_tree=None, force=False):
         pipe.append('-f')
     pipe.append(commit_hash)
     result = command.run_pipe([pipe], capture=True, raise_on_error=False,
-                             capture_stderr=True)
+                              capture_stderr=True)
     if result.return_code != 0:
         raise OSError('git checkout (%s): %s' % (pipe, result.stderr))
 
+
 def clone(git_dir, output_dir):
     """Checkout the selected commit for this build
 
@@ -243,10 +251,11 @@ def clone(git_dir, output_dir):
     """
     pipe = ['git', 'clone', git_dir, '.']
     result = command.run_pipe([pipe], capture=True, cwd=output_dir,
-                             capture_stderr=True)
+                              capture_stderr=True)
     if result.return_code != 0:
         raise OSError('git clone: %s' % result.stderr)
 
+
 def fetch(git_dir=None, work_tree=None):
     """Fetch from the origin repo
 
@@ -263,6 +272,7 @@ def fetch(git_dir=None, work_tree=None):
     if result.return_code != 0:
         raise OSError('git fetch: %s' % result.stderr)
 
+
 def check_worktree_is_available(git_dir):
     """Check if git-worktree functionality is available
 
@@ -274,9 +284,10 @@ def check_worktree_is_available(git_dir):
     """
     pipe = ['git', '--git-dir', git_dir, 'worktree', 'list']
     result = command.run_pipe([pipe], capture=True, capture_stderr=True,
-                             raise_on_error=False)
+                              raise_on_error=False)
     return result.return_code == 0
 
+
 def add_worktree(git_dir, output_dir, commit_hash=None):
     """Create and checkout a new git worktree for this build
 
@@ -290,10 +301,11 @@ def add_worktree(git_dir, output_dir, commit_hash=None):
     if commit_hash:
         pipe.append(commit_hash)
     result = command.run_pipe([pipe], capture=True, cwd=output_dir,
-                             capture_stderr=True)
+                              capture_stderr=True)
     if result.return_code != 0:
         raise OSError('git worktree add: %s' % result.stderr)
 
+
 def prune_worktrees(git_dir):
     """Remove administrative files for deleted worktrees
 
@@ -305,7 +317,8 @@ def prune_worktrees(git_dir):
     if result.return_code != 0:
         raise OSError('git worktree prune: %s' % result.stderr)
 
-def create_patches(branch, start, count, ignore_binary, series, signoff = True):
+
+def create_patches(branch, start, count, ignore_binary, series, signoff=True):
     """Create a series of patches from the top of the current branch.
 
     The patch files are written to the current directory using
@@ -321,9 +334,7 @@ def create_patches(branch, start, count, ignore_binary, series, signoff = True):
         Filename of cover letter (None if none)
         List of filenames of patch files
     """
-    if series.get('version'):
-        version = '%s ' % series['version']
-    cmd = ['git', 'format-patch', '-M' ]
+    cmd = ['git', 'format-patch', '-M']
     if signoff:
         cmd.append('--signoff')
     if ignore_binary:
@@ -341,9 +352,10 @@ def create_patches(branch, start, count, ignore_binary, series, signoff = True):
 
     # We have an extra file if there is a cover letter
     if series.get('cover'):
-       return files[0], files[1:]
+        return files[0], files[1:]
     else:
-       return None, files
+        return None, files
+
 
 def build_email_list(in_list, tag=None, alias=None, warn_on_error=True):
     """Build a list of email addresses based on an input list.
@@ -385,40 +397,43 @@ def build_email_list(in_list, tag=None, alias=None, warn_on_error=True):
         raw += lookup_email(item, alias, warn_on_error=warn_on_error)
     result = []
     for item in raw:
-        if not item in result:
+        if item not in result:
             result.append(item)
     if tag:
         return ['%s %s%s%s' % (tag, quote, email, quote) for email in result]
     return result
 
+
 def check_suppress_cc_config():
     """Check if sendemail.suppresscc is configured correctly.
 
     Returns:
         True if the option is configured correctly, False otherwise.
     """
-    suppresscc = command.output_one_line('git', 'config', 'sendemail.suppresscc',
-                                       raise_on_error=False)
+    suppresscc = command.output_one_line(
+        'git', 'config', 'sendemail.suppresscc', raise_on_error=False)
 
     # Other settings should be fine.
     if suppresscc == 'all' or suppresscc == 'cccmd':
         col = terminal.Color()
 
         print((col.build(col.RED, "error") +
-            ": git config sendemail.suppresscc set to %s\n"  % (suppresscc)) +
-            "  patman needs --cc-cmd to be run to set the cc list.\n" +
-            "  Please run:\n" +
-            "    git config --unset sendemail.suppresscc\n" +
-            "  Or read the man page:\n" +
-            "    git send-email --help\n" +
-            "  and set an option that runs --cc-cmd\n")
+               ": git config sendemail.suppresscc set to %s\n"
+               % (suppresscc)) +
+              "  patman needs --cc-cmd to be run to set the cc list.\n" +
+              "  Please run:\n" +
+              "    git config --unset sendemail.suppresscc\n" +
+              "  Or read the man page:\n" +
+              "    git send-email --help\n" +
+              "  and set an option that runs --cc-cmd\n")
         return False
 
     return True
 
+
 def email_patches(series, cover_fname, args, dry_run, warn_on_error, cc_fname,
-        self_only=False, alias=None, in_reply_to=None, thread=False,
-        smtp_server=None):
+                  self_only=False, alias=None, in_reply_to=None, thread=False,
+                  smtp_server=None):
     """Email a patch series.
 
     Args:
@@ -487,9 +502,10 @@ send --cc-cmd cc-fname" cover p1 p2'
                   "git config sendemail.to u-boot@lists.denx.de")
             return
     cc = build_email_list(list(set(series.get('cc')) - set(series.get('to'))),
-                        '--cc', alias, warn_on_error)
+                          '--cc', alias, warn_on_error)
     if self_only:
-        to = build_email_list([os.getenv('USER')], '--to', alias, warn_on_error)
+        to = build_email_list([os.getenv('USER')], '--to',
+                              alias, warn_on_error)
         cc = []
     cmd = ['git', 'send-email', '--annotate']
     if smtp_server:
@@ -565,7 +581,7 @@ def lookup_email(lookup_name, alias=None, warn_on_error=True, level=0):
     if not alias:
         alias = settings.alias
     lookup_name = lookup_name.strip()
-    if '@' in lookup_name: # Perhaps a real email address
+    if '@' in lookup_name:      # Perhaps a real email address
         return [lookup_name]
 
     lookup_name = lookup_name.lower()
@@ -581,7 +597,7 @@ def lookup_email(lookup_name, alias=None, warn_on_error=True, level=0):
             return out_list
 
     if lookup_name:
-        if not lookup_name in alias:
+        if lookup_name not in alias:
             msg = "Alias '%s' not found" % lookup_name
             if warn_on_error:
                 print(col.build(col.RED, msg))
@@ -589,11 +605,12 @@ def lookup_email(lookup_name, alias=None, warn_on_error=True, level=0):
         for item in alias[lookup_name]:
             todo = lookup_email(item, alias, warn_on_error, level + 1)
             for new_item in todo:
-                if not new_item in out_list:
+                if new_item not in out_list:
                     out_list.append(new_item)
 
     return out_list
 
+
 def get_top_level():
     """Return name of top-level directory for this git repo.
 
@@ -608,6 +625,7 @@ def get_top_level():
     """
     return command.output_one_line('git', 'rev-parse', '--show-toplevel')
 
+
 def get_alias_file():
     """Gets the name of the git alias file.
 
@@ -615,7 +633,7 @@ def get_alias_file():
         Filename of git alias file, or None if none
     """
     fname = command.output_one_line('git', 'config', 'sendemail.aliasesfile',
-            raise_on_error=False)
+                                    raise_on_error=False)
     if not fname:
         return None
 
@@ -625,6 +643,7 @@ def get_alias_file():
 
     return os.path.join(get_top_level(), fname)
 
+
 def get_default_user_name():
     """Gets the user.name from .gitconfig file.
 
@@ -634,6 +653,7 @@ def get_default_user_name():
     uname = command.output_one_line('git', 'config', '--global', 'user.name')
     return uname
 
+
 def get_default_user_email():
     """Gets the user.email from the global .gitconfig file.
 
@@ -643,17 +663,19 @@ def get_default_user_email():
     uemail = command.output_one_line('git', 'config', '--global', 'user.email')
     return uemail
 
+
 def get_default_subject_prefix():
     """Gets the format.subjectprefix from local .git/config file.
 
     Returns:
         Subject prefix found in local .git/config file, or None if none
     """
-    sub_prefix = command.output_one_line('git', 'config', 'format.subjectprefix',
-                 raise_on_error=False)
+    sub_prefix = command.output_one_line(
+        'git', 'config', 'format.subjectprefix', raise_on_error=False)
 
     return sub_prefix
 
+
 def setup():
     """Set up git utils, by reading the alias files."""
     # Check for a git alias file also
@@ -666,6 +688,7 @@ def setup():
     use_no_decorate = (command.run_pipe([cmd], raise_on_error=False)
                        .return_code == 0)
 
+
 def get_head():
     """Get the hash of the current HEAD
 
@@ -674,6 +697,7 @@ def get_head():
     """
     return command.output_one_line('git', 'show', '-s', '--pretty=format:%H')
 
+
 if __name__ == "__main__":
     import doctest
 

base-commit: 9bd3d354a1a0712ac27c717df9ad60566b0406ee
-- 
2.38.1

[PATCH v2 2/8] patman: locate test data files via __file__ and pathlib

[Maxim Cournoyer]

Previously it would rely on the executing script location, which could
break for example when running the tests via 'pytest'.

Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
---

(no changes since v1)

 tools/patman/func_test.py | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py
index 7b92bc67be..7fa4a00786 100644
--- a/tools/patman/func_test.py
+++ b/tools/patman/func_test.py
@@ -7,6 +7,7 @@
 """Functional tests for checking that patman behaves correctly"""
 
 import os
+import pathlib
 import re
 import shutil
 import sys
@@ -28,6 +29,10 @@ from patman.test_util import capture_sys_output
 import pygit2
 from patman import status
 
+
+TEST_DATA_DIR = pathlib.Path(__file__).parent / 'test/'
+
+
 class TestFunctional(unittest.TestCase):
     """Functional tests for checking that patman behaves correctly"""
     leb = (b'Lord Edmund Blackadd\xc3\xabr <weasel@blackadder.org>'.
@@ -57,8 +62,7 @@ class TestFunctional(unittest.TestCase):
         Returns:
             str: Full path to file in the test directory
         """
-        return os.path.join(os.path.dirname(os.path.realpath(sys.argv[0])),
-                            'test', fname)
+        return TEST_DATA_DIR / fname
 
     @classmethod
     def _get_text(cls, fname):
-- 
2.38.1

[PATCH v2 3/8] patman: invoke the checkpatch.pl script with '--u-boot' and '--strict'

[Maxim Cournoyer]

This resolves 10 out of 11 test failures seen when running './patman
test' from the 'tools/patman' subdirectory. This was caused by the
.checkpatch.conf configuration file at the root of the project not
being picked up. Make the test suite of patman independant from it by
always invoking the checkpatch.pl script with the minimally required
arguments for the test suite to pass.

Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
---

Changes in v2:
- Reword the commit message with a better explanation of the problem
- Add the '--strict' argument to script invocation

 tools/patman/checkpatch.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py
index d1b902dd96..012c0d895c 100644
--- a/tools/patman/checkpatch.py
+++ b/tools/patman/checkpatch.py
@@ -211,7 +211,7 @@ def check_patch(fname, verbose=False, show_types=False, use_tree=False):
             stdout: Full output of checkpatch
     """
     chk = find_check_patch()
-    args = [chk]
+    args = [chk, '--u-boot', '--strict']
     if not use_tree:
         args.append('--no-tree')
     if show_types:
-- 
2.38.1

[PATCH v2 4/8] patman: rename main script to __main__.py

[Maxim Cournoyer]

This allows running the package as a Python module, like e.g.:

    $ python -m patman

It also prevents Pytest from attempting to parse main.py, which
would cause errors.

Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
---

(no changes since v1)

 tools/patman/{main.py => __main__.py} | 0
 tools/patman/patman                   | 2 +-
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename tools/patman/{main.py => __main__.py} (100%)

diff --git a/tools/patman/main.py b/tools/patman/__main__.py
similarity index 100%
rename from tools/patman/main.py
rename to tools/patman/__main__.py
diff --git a/tools/patman/patman b/tools/patman/patman
index 11a5d8e18a..5a427d1942 120000
--- a/tools/patman/patman
+++ b/tools/patman/patman
@@ -1 +1 @@
-main.py
\ No newline at end of file
+__main__.py
\ No newline at end of file
-- 
2.38.1

[PATCH v2 5/8] patman: add pytest configuration file

[Maxim Cournoyer]

With this change, a user can run the patman test suite using Pytest
the same as when using 'patman test':

    $ cd tools/patman && pytest
    [...]
    44 passed, 8 warnings in 8.87s

    $ ./patman test
    Ran 44 tests in 8.460s

Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
---

(no changes since v1)

 tools/patman/pytest.ini | 2 ++
 1 file changed, 2 insertions(+)
 create mode 100644 tools/patman/pytest.ini

diff --git a/tools/patman/pytest.ini b/tools/patman/pytest.ini
new file mode 100644
index 0000000000..df3eb518d0
--- /dev/null
+++ b/tools/patman/pytest.ini
@@ -0,0 +1,2 @@
+[pytest]
+addopts = --doctest-modules
-- 
2.38.1

[PATCH v2 6/8] patman: hide the 'test' action unless test data is available

[Maxim Cournoyer]

Some tests would fail when the test data is not available, so it
doesn't make much sense to expose the action when patman is running
outside of the u-boot git checkout.

Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
---

(no changes since v1)

 tools/patman/__main__.py | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tools/patman/__main__.py b/tools/patman/__main__.py
index 5a7756a221..82cef4fc0b 100755
--- a/tools/patman/__main__.py
+++ b/tools/patman/__main__.py
@@ -21,6 +21,7 @@ if __name__ == "__main__":
 # Our modules
 from patman import command
 from patman import control
+from patman import func_test
 from patman import gitutil
 from patman import project
 from patman import settings
@@ -96,9 +97,11 @@ send.add_argument('--smtp-server', type=str,
 
 send.add_argument('patchfiles', nargs='*')
 
-test_parser = subparsers.add_parser('test', help='Run tests')
-test_parser.add_argument('testname', type=str, default=None, nargs='?',
-                         help="Specify the test to run")
+# Only add the 'test' action if the test data files are available.
+if os.path.exists(func_test.TEST_DATA_DIR):
+    test_parser = subparsers.add_parser('test', help='Run tests')
+    test_parser.add_argument('testname', type=str, default=None, nargs='?',
+                             help="Specify the test to run")
 
 status = subparsers.add_parser('status',
                                help='Check status of patches in patchwork')
-- 
2.38.1

[PATCH v2 7/8] patman: document how to run test suite via pytest

[Maxim Cournoyer]

Pytest offers useful features such as selecting tests by means of a
regular expression, or running the pdb debugger upon encountering a
test failure.

Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
---

(no changes since v1)

 tools/patman/patman.rst | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/patman/patman.rst b/tools/patman/patman.rst
index 8c5c9cc2cc..b06399b459 100644
--- a/tools/patman/patman.rst
+++ b/tools/patman/patman.rst
@@ -680,6 +680,12 @@ them:
 
     $ tools/patman/patman test
 
+Alternatively, you can run the test suite via Pytest:
+
+.. code-block:: bash
+
+    $ cd tools/patman && pytest
+
 Error handling doesn't always produce friendly error messages - e.g.
 putting an incorrect tag in a commit may provide a confusing message.
 
-- 
2.38.1

[PATCH v2 8/8] patman: document default 'send' command

[Maxim Cournoyer]

Document that this command is the default and what it's intended for.

Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
---

(no changes since v1)

 tools/patman/__main__.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/patman/__main__.py b/tools/patman/__main__.py
index 82cef4fc0b..11f19281fb 100755
--- a/tools/patman/__main__.py
+++ b/tools/patman/__main__.py
@@ -56,7 +56,8 @@ parser.add_argument('-H', '--full-help', action='store_true', dest='full_help',
                     default=False, help='Display the README file')
 
 subparsers = parser.add_subparsers(dest='cmd')
-send = subparsers.add_parser('send')
+send = subparsers.add_parser(
+    'send', help='Format, check and email patches (default command)')
 send.add_argument('-i', '--ignore-errors', action='store_true',
        dest='ignore_errors', default=False,
        help='Send patches email even if patch errors are found')
-- 
2.38.1

Re: [PATCH v2 1/8] patman: cosmetic: Fix PEP 8 warnings for the gitutil module.

[Simon Glass]

On Mon, 19 Dec 2022 at 07:53, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
> This patch fixes all the PEP 8 warnings reported by Pyflake for the
> gitutil module.
>
> Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
> ---
>
> (no changes since v1)
>
>  tools/patman/gitutil.py | 106 ++++++++++++++++++++++++----------------
>  1 file changed, 65 insertions(+), 41 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

BTW we do have a 'make pylint' target which finds more problems (with pylint).

Re: [PATCH v2 2/8] patman: locate test data files via __file__ and pathlib

[Simon Glass]

On Mon, 19 Dec 2022 at 07:53, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
> Previously it would rely on the executing script location, which could
> break for example when running the tests via 'pytest'.
>
> Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
> ---
>
> (no changes since v1)
>
>  tools/patman/func_test.py | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH v2 3/8] patman: invoke the checkpatch.pl script with '--u-boot' and '--strict'

[Simon Glass]

On Mon, 19 Dec 2022 at 07:53, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
> This resolves 10 out of 11 test failures seen when running './patman
> test' from the 'tools/patman' subdirectory. This was caused by the
> .checkpatch.conf configuration file at the root of the project not
> being picked up. Make the test suite of patman independant from it by

independent

> always invoking the checkpatch.pl script with the minimally required
> arguments for the test suite to pass.
>
> Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
> ---
>
> Changes in v2:
> - Reword the commit message with a better explanation of the problem
> - Add the '--strict' argument to script invocation
>
>  tools/patman/checkpatch.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH v2 4/8] patman: rename main script to __main__.py

[Simon Glass]

On Mon, 19 Dec 2022 at 07:53, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
> This allows running the package as a Python module, like e.g.:
>
>     $ python -m patman
>
> It also prevents Pytest from attempting to parse main.py, which
> would cause errors.
>
> Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
> ---
>
> (no changes since v1)
>
>  tools/patman/{main.py => __main__.py} | 0
>  tools/patman/patman                   | 2 +-
>  2 files changed, 1 insertion(+), 1 deletion(-)
>  rename tools/patman/{main.py => __main__.py} (100%)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH v2 5/8] patman: add pytest configuration file

[Simon Glass]

On Mon, 19 Dec 2022 at 07:53, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
> With this change, a user can run the patman test suite using Pytest
> the same as when using 'patman test':
>
>     $ cd tools/patman && pytest
>     [...]
>     44 passed, 8 warnings in 8.87s
>
>     $ ./patman test
>     Ran 44 tests in 8.460s
>
> Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
> ---
>
> (no changes since v1)
>
>  tools/patman/pytest.ini | 2 ++
>  1 file changed, 2 insertions(+)
>  create mode 100644 tools/patman/pytest.ini

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH v2 6/8] patman: hide the 'test' action unless test data is available

[Simon Glass]

On Mon, 19 Dec 2022 at 07:53, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
> Some tests would fail when the test data is not available, so it
> doesn't make much sense to expose the action when patman is running
> outside of the u-boot git checkout.
>
> Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
> ---
>
> (no changes since v1)
>
>  tools/patman/__main__.py | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

This really should have an update in the docs

Re: [PATCH v2 7/8] patman: document how to run test suite via pytest

[Simon Glass]

On Mon, 19 Dec 2022 at 07:53, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
> Pytest offers useful features such as selecting tests by means of a
> regular expression, or running the pdb debugger upon encountering a
> test failure.
>
> Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
> ---
>
> (no changes since v1)
>
>  tools/patman/patman.rst | 6 ++++++
>  1 file changed, 6 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>


>
> diff --git a/tools/patman/patman.rst b/tools/patman/patman.rst
> index 8c5c9cc2cc..b06399b459 100644
> --- a/tools/patman/patman.rst
> +++ b/tools/patman/patman.rst
> @@ -680,6 +680,12 @@ them:
>
>      $ tools/patman/patman test
>
> +Alternatively, you can run the test suite via Pytest:
> +
> +.. code-block:: bash
> +
> +    $ cd tools/patman && pytest
> +
>  Error handling doesn't always produce friendly error messages - e.g.
>  putting an incorrect tag in a commit may provide a confusing message.
>
> --
> 2.38.1
>

Re: [PATCH v2 8/8] patman: document default 'send' command

[Simon Glass]

On Mon, 19 Dec 2022 at 07:53, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
> Document that this command is the default and what it's intended for.
>
> Signed-off-by: Maxim Cournoyer <maxim.cournoyer@savoirfairelinux.com>
> ---
>
> (no changes since v1)
>
>  tools/patman/__main__.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>