[tests] add python linter (#300)

* [tests] add python linter

* [pylint] add pylint dev routine

 - pre-commit
 - CI

* Bump ledger-app-dev-tools version to 3.42.0

* [test] fix pytezos version for python 3.9

 - fix pytezos version to the latest version compatible with python 3.9

.github/workflows/build.yml

@@ -89,6 +89,7 @@ jobs:
             apk add gmp-dev curl jq libsodium-dev git xxd procps
             python3 -m venv tezos_test_env --system-site-package
             source ./tezos_test_env/bin/activate
+            python3 -m pip install --upgrade pip -q
             python3 -m pip install -r ./tests/requirements.txt -q
             TMP_DIR=$(mktemp -d /tmp/foo-XXXXXX)
             tar xfz app_${{ matrix.device }}_dbg.tgz -C $TMP_DIR
@@ -192,6 +193,7 @@ jobs:
             apk add gmp-dev curl jq libsodium-dev git xxd procps
             python3 -m venv tezos_test_env --system-site-package
             source ./tezos_test_env/bin/activate
+            python3 -m pip install --upgrade pip -q
             python3 -m pip install -r ./tests/requirements.txt -q
             ./tests/integration/run_test_local.sh -T100 -F -m ${{ matrix.device }} tests
 

.github/workflows/coding_style_checks.yml

@@ -23,3 +23,20 @@ jobs:
       source: './app/src'
       extensions: 'h,c'
       version: 15
+
+  pylint_check:
+    name: Python Lint Check
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+
+      - name: Set up Python
+        uses: actions/setup-python@v2
+        with:
+          python-version: '3.9'
+
+      - name: Install pylint
+        run: pip install pylint -r tests/requirements.txt
+
+      - name: Run pylint with .pylintrc
+        run: pylint --errors-only --rcfile=tests/integration/.pylintrc tests/integration/nano/

.github/workflows/pre-commit.yml

@@ -12,4 +12,4 @@ jobs:
           python-version: '3.10'
       - run: python -m pip install pre-commit
       - run: python -m pip freeze --local
-      - run: SKIP=clang-format pre-commit run --all-files
+      - run: SKIP=clang-format,pylint pre-commit run --all-files

.pre-commit-config.yaml

@@ -19,3 +19,12 @@ repos:
         language: system
         files: ^
         types: [file, c]
+
+      - id: pylint
+        name: pylint
+        description: Run pylint with custom .pylintrc configuration
+        entry: pylint
+        language: system
+        pass_filenames: false
+        args: ["--errors-only", "--rcfile=tests/integration/.pylintrc", "tests/integration/nano/"]
+        types: [python]

Makefile

@@ -122,6 +122,7 @@ integration_tests_basic_%:	app_%.tgz   \
 		apk add gmp-dev curl jq libsodium-dev git xxd procps;     \
 		python3 -m venv tezos_test_env --system-site-package;     \
 		source ./tezos_test_env/bin/activate;                     \
+		python3 -m pip install --upgrade pip -q;                  \
 		python3 -m pip install -r tests/requirements.txt -q ;  \
 		python3 -m pytest -n 32 tests/integration/nano/ --tb=no   \
 			--device $* --app \$$TMP_DIR/app.elf                  \

scripts/test_swap.sh

@@ -151,7 +151,8 @@ run_tests() {
                --rm -v "$(realpath .):/app"                             \
                ledger-app-tezos-integration-tests -c                    \
                "cd /app &&                                              \
-                pip install -r test/python/requirements.txt -q &&    \
+                pip install --upgrade pip -q &&                         \
+                pip install -r test/python/requirements.txt -q &&       \
                 pip install protobuf==3.20.3 && pytest test/python $*"
     )
 }

tests/integration/.pylintrc

@@ -0,0 +1,12 @@
+[MESSAGES CONTROL]
+disable=
+    broad-exception-caught,
+    global-statement,
+    line-too-long,
+    protected-access,
+    redefined-outer-name,
+    too-few-public-methods,
+    too-many-arguments,
+    too-many-instance-attributes,
+    too-many-positional-arguments,
+    too-many-statements,

tests/integration/nano/conftest.py

@@ -1,11 +1,29 @@
-import pytest
+# Copyright 2024 Trilitech <[email protected]>
+# Copyright 2024 Functori <[email protected]>
+
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+
+# http://www.apache.org/licenses/LICENSE-2.0
+
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Conftest base on `ragger` conftest."""
+
 
 from pathlib import Path
+from typing import Dict, Generator, List, Union
+
+import pytest
 from ragger.firmware import Firmware
-from typing import Dict, Callable, Generator, List, Optional, Tuple, Union
 
 from utils.app import TezosAppScreen, SpeculosTezosBackend, DEFAULT_SEED
-from utils.backend import TezosBackend, APP_KIND
+from utils.backend import AppKind
 
 FIRMWARES: List[Firmware] = [
     Firmware.NANOS,
@@ -16,6 +34,7 @@
 DEVICES: List[str] = list(map(lambda fw: fw.device, FIRMWARES))
 
 def pytest_addoption(parser):
+    """Register argparse-style options for pytest."""
     parser.addoption("-D", "--device",
                      type=str,
                      choices=DEVICES,
@@ -49,6 +68,7 @@ def pytest_addoption(parser):
 global_log_dir: Union[Path, None] = None
 
 def pytest_configure(config):
+    """Configure pytest."""
     global global_log_dir
     log_dir = config.getoption("log_dir")
     if log_dir is not None:
@@ -57,21 +77,24 @@ def pytest_configure(config):
 logs : Dict[str, List[pytest.TestReport]] = {}
 
 @pytest.hookimpl(tryfirst=True)
-def pytest_runtest_logstart(nodeid, location):
+def pytest_runtest_logstart(location):
+    """Called at the start of running the runtest protocol for a single item."""
     logs[location[2]] = []
 
 @pytest.hookimpl(tryfirst=True)
 def pytest_runtest_logreport(report):
+    """Called at the end of running the runtest protocol for a single test."""
     logs[report.head_line].append(report)
 
 @pytest.hookimpl(tryfirst=True)
-def pytest_runtest_logfinish(nodeid, location):
+def pytest_runtest_logfinish(location):
+    """Called at the end of running the runtest protocol for a single item."""
     if global_log_dir is not None:
         log_dir = global_log_dir / Path(location[0]).stem
         log_dir.mkdir(parents=True, exist_ok=True)
         head_line = location[2]
         log_file = log_dir / f"{head_line.replace(' ', '_')}.log"
-        with open(log_file, 'w') as writer:
+        with open(log_file, 'w', encoding="utf-8") as writer:
             for report in logs[head_line]:
                 writer.write(f"============================== {report.when.capitalize()} {report.outcome} ==============================\n")
                 writer.write(f"{report.longreprtext}\n")
@@ -84,37 +107,44 @@ def pytest_runtest_logfinish(nodeid, location):
 
 @pytest.fixture(scope="session")
 def firmware(pytestconfig) -> Firmware :
+    """Get `firware` for pytest."""
     device = pytestconfig.getoption("device")
     return next(fw for fw in FIRMWARES if fw.device == device)
 
 @pytest.fixture(scope="session")
-def port(pytestconfig, request, worker_id) -> int :
+def port(pytestconfig, worker_id) -> int :
+    """Get `port` for pytest."""
     if worker_id == "master":
         return pytestconfig.getoption("port")
     # worker_id = gw0, gw1, ...
     return 5000 + int(worker_id[2:])
 
 @pytest.fixture(scope="session")
 def display(pytestconfig) -> bool :
+    """Get `display` for pytest."""
     return pytestconfig.getoption("display")
 
 @pytest.fixture(scope="session")
 def golden_run(pytestconfig) -> bool:
+    """Get `golden_run` for pytest."""
     return pytestconfig.getoption("golden_run")
 
 @pytest.fixture(scope="session")
 def app_path(pytestconfig) -> Path:
+    """Get `app_path` for pytest."""
     return Path(pytestconfig.getoption("app"))
 
 @pytest.fixture(scope="session")
 def speculos_args(pytestconfig) -> List[str]:
+    """Get `app_path` for pytest."""
     speculos_args = pytestconfig.getoption("speculos_args")
     if speculos_args is None:
         return []
     return speculos_args.split()
 
 @pytest.fixture(scope="function")
 def seed(request) -> str:
+    """Get `seed` for pytest."""
     param = getattr(request, "param", None)
     return param.get("seed", DEFAULT_SEED) if param else DEFAULT_SEED
 
@@ -125,6 +155,7 @@ def backend(app_path: Path,
             display: bool,
             seed: str,
             speculos_args: List[str]) -> Generator[SpeculosTezosBackend, None, None]:
+    """Get `backend` for pytest."""
 
     if display:
         speculos_args += ["--display", "qt"]
@@ -143,10 +174,12 @@ def backend(app_path: Path,
         yield b
 
 @pytest.fixture(scope="function")
-def app(backend: SpeculosTezosBackend, golden_run: bool):
-    return TezosAppScreen(backend, APP_KIND.WALLET, golden_run)
+def app(backend: SpeculosTezosBackend, golden_run: bool) -> TezosAppScreen:
+    """Get `app` for pytest."""
+    return TezosAppScreen(backend, AppKind.WALLET, golden_run)
 
 def requires_device(device):
+    """Wrapper to run the pytest test only with the provided device."""
     return pytest.mark.skipif(
         f"config.getvalue('device') != '{ device }'",
         reason=f"Test requires device to be { device }."

tests/integration/nano/nanos/test_nanos_regression_batched_ops.py

@@ -13,19 +13,13 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import os
-import sys
+"""Check signing batch operation"""
 
-file_path=os.path.abspath(__file__)
-dir_path=os.path.dirname(file_path)
-root_path=os.path.dirname(dir_path)
-sys.path.append(root_path)
+from pathlib import Path
 
 from conftest import requires_device
 
-from pathlib import Path
-
-from utils.app import Screen, DEFAULT_ACCOUNT
+from utils.app import TezosAppScreen, DEFAULT_ACCOUNT
 from utils.message import Message
 
 # Operation (0): Transaction
@@ -42,7 +36,8 @@
 # Destination: KT1CYT8oACUcCSNTu2qfgB4fj5bD7szYrpti
 
 @requires_device("nanos")
-def test_nanos_regression_batched_ops(app):
+def test_nanos_regression_batched_ops(app: TezosAppScreen):
+    """Check signing batch operation"""
     test_name = Path(__file__).stem
 
     app.setup_expert_mode()

tests/integration/nano/nanosp/test_nanosp_regression_potential_empty_screen.py

@@ -13,19 +13,13 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import os
-import sys
+"""Check signing operation that display potentially empty screens"""
 
-file_path=os.path.abspath(__file__)
-dir_path=os.path.dirname(file_path)
-root_path=os.path.dirname(dir_path)
-sys.path.append(root_path)
+from pathlib import Path
 
 from conftest import requires_device
 
-from pathlib import Path
-
-from utils.app import Screen, DEFAULT_ACCOUNT
+from utils.app import TezosAppScreen, DEFAULT_ACCOUNT
 from utils.message import Message
 
 # Operation (0): Transfer ticket
@@ -43,7 +37,8 @@
 # S
 
 @requires_device("nanosp")
-def test_nanosp_regression_potential_empty_screen(app):
+def test_nanosp_regression_potential_empty_screen(app: TezosAppScreen):
+    """Check signing operation that display potentially empty screens"""
     test_name = Path(__file__).stem
 
     app.setup_expert_mode()

tests/integration/nano/nanosp/test_nanosp_regression_press_right_works_across_apdu_recieves.py

@@ -13,28 +13,23 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import os
-import sys
+"""Check no need to click right two times between APDUs during signing flow"""
 
-file_path=os.path.abspath(__file__)
-dir_path=os.path.dirname(file_path)
-root_path=os.path.dirname(dir_path)
-sys.path.append(root_path)
+from pathlib import Path
 
 from conftest import requires_device
 
-from pathlib import Path
-
-from utils.app import Screen, DEFAULT_ACCOUNT
+from utils.app import Screen, TezosAppScreen, DEFAULT_ACCOUNT
 from utils.message import Message
 
 # Expression: {IF_NONE {{SWAP;IF {DIP {{DROP 1;PUSH unit Unit;PUSH bool True;PUSH string ";L\\S?p$-Fq)VDg\n]te\no4v0_8)\""}}} {{DROP 2;PUSH unit Unit;PUSH bool False;PUSH string "Li-%*edF6~?E[5Kmu?dyviwJ^2\"\\d$FyQ>>!>D$g(Qg";PUSH string "*Tx<E`SiG6Yf*A^kZ\\=7?H[mOlQ\n]Ehs"}}}} {IF_NONE {DUP} {{DROP 4;PUSH unit Unit;PUSH bool True;PUSH string "\"\\6_4\n$k%";PUSH string "c^1\"\\?Ey_1!EVb~9;EX;YU\n#Kj2ZT8h`U!X "}}};SIZE}
 
 @requires_device("nanosp")
-def test_nanosp_regression_press_right_works_across_apdu_recieves(app):
+def test_nanosp_regression_press_right_works_across_apdu_recieves(app: TezosAppScreen):
+    """Check no need to click right two times between APDUs during signing flow"""
     test_name = Path(__file__).stem
 
-    app.assert_screen(Screen.Home)
+    app.assert_screen(Screen.HOME)
 
     message = Message.from_bytes("050200000135072f02000000c502000000c0034c072c0200000040051f02000000390200000034052000010743036c030b07430359030a07430368010000001b3b4c5c533f70242d4671295644670a5d74650a6f3476305f3829220200000072020000006d052000020743036c030b07430359030307430368010000002b4c692d252a656446367e3f455b354b6d753f64797669774a5e32225c64244679513e3e213e4424672851670743036801000000202a54783c45605369473659662a415e6b5a5c3d373f485b6d4f6c510a5d4568730200000062072f020000000203210200000054020000004f052000040743036c030b07430359030a074303680100000009225c365f340a246b25074303680100000024635e31225c3f45795f31214556627e393b45583b59550a234b6a325a54386860552158200345")
 

tests/integration/nano/nanox/test_nanox_regression_batched_ops.py

@@ -13,19 +13,13 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import os
-import sys
+"""Check signing batch operation"""
 
-file_path=os.path.abspath(__file__)
-dir_path=os.path.dirname(file_path)
-root_path=os.path.dirname(dir_path)
-sys.path.append(root_path)
+from pathlib import Path
 
 from conftest import requires_device
 
-from pathlib import Path
-
-from utils.app import Screen, DEFAULT_ACCOUNT
+from utils.app import TezosAppScreen, DEFAULT_ACCOUNT
 from utils.message import Message
 
 # Operation (0): Transaction
@@ -42,7 +36,8 @@
 # Destination: KT1CYT8oACUcCSNTu2qfgB4fj5bD7szYrpti
 
 @requires_device("nanox")
-def test_nanox_regression_batched_ops(app):
+def test_nanox_regression_batched_ops(app: TezosAppScreen):
+    """Check signing batch operation"""
     test_name = Path(__file__).stem
 
     app.setup_expert_mode()