From cf3c5c3064aa4596d89c91bb53e2604b05141029 Mon Sep 17 00:00:00 2001 From: Charles-Edouard de la Vergne Date: Fri, 21 Jun 2024 14:03:31 +0200 Subject: [PATCH] Improve ragger tests - Use defined firmware instead of strings - Fix compatibility with Python 3.9 - Fix test navigation steps --- tests/ragger/conftest.py | 5 +++-- tests/ragger/setup.cfg | 3 +++ tests/ragger/test_blind_sign.py | 22 ++++++++++------------ tests/ragger/test_domain_name.py | 29 ++++++++++++----------------- tests/ragger/test_eip191.py | 29 +++++++++++------------------ tests/ragger/test_eip712.py | 10 +++++----- tests/ragger/test_get_address.py | 14 +++++--------- tests/ragger/test_nft.py | 25 +++++++------------------ tests/ragger/test_sign.py | 17 ++++++----------- 9 files changed, 62 insertions(+), 92 deletions(-) diff --git a/tests/ragger/conftest.py b/tests/ragger/conftest.py index 2408765..086d95e 100644 --- a/tests/ragger/conftest.py +++ b/tests/ragger/conftest.py @@ -1,5 +1,6 @@ import sys from pathlib import Path +from os import path import warnings import glob @@ -17,8 +18,8 @@ def pytest_addoption(parser): parser.addoption("--with_lib_mode", action="store_true", help="Run the test with Library Mode") -parent: Path = Path(__file__).parent -testFiles = glob.glob("test_*.py", root_dir=f"{parent}") +pattern = f"{Path(__file__).parent}/test_*.py" +testFiles = [path.basename(x) for x in glob.glob(pattern)] collect_ignore = [] if "--with_lib_mode" in sys.argv: diff --git a/tests/ragger/setup.cfg b/tests/ragger/setup.cfg index d7f3b7b..89bf1ed 100644 --- a/tests/ragger/setup.cfg +++ b/tests/ragger/setup.cfg @@ -8,6 +8,9 @@ disable = C0114, # missing-module-docstring C0103, # invalid-name R0801, # duplicate-code R0903, # too-few-public-methods + R0904, # too-many-public-methods + R0911, # too-many-statements + R0912, # too-many-branches R0913, # too-many-arguments R0914, # too-many-locals W0603, # global-statement diff --git a/tests/ragger/test_blind_sign.py b/tests/ragger/test_blind_sign.py index c058b9f..0885389 100644 --- a/tests/ragger/test_blind_sign.py +++ b/tests/ragger/test_blind_sign.py @@ -7,7 +7,6 @@ from web3 import Web3 from ragger.backend import BackendInterface from ragger.firmware import Firmware from ragger.navigator import Navigator, NavInsID -from ragger.navigator.navigation_scenario import NavigateWithScenario from ragger.error import ExceptionRAPDU from constants import ABIS_FOLDER @@ -77,8 +76,8 @@ def test_blind_sign(firmware: Firmware, test_name += "_rejected" moves = [] - if firmware.device.startswith("nano"): - if firmware.device == "nanos": + if firmware.is_nano: + if firmware == Firmware.NANOS: moves += [NavInsID.RIGHT_CLICK] * 2 else: moves += [NavInsID.RIGHT_CLICK] * 4 @@ -89,13 +88,13 @@ def test_blind_sign(firmware: Firmware, moves += [NavInsID.BOTH_CLICK] if sign: - if firmware.device == "nanos": + if firmware == Firmware.NANOS: moves += [NavInsID.RIGHT_CLICK] * 10 else: moves += [NavInsID.RIGHT_CLICK] * 6 moves += [NavInsID.BOTH_CLICK] else: - if firmware.device == "stax": + if firmware == Firmware.STAX: tap_number = 2 else: tap_number = 3 @@ -126,7 +125,7 @@ def test_blind_sign_reject_in_risk_review(firmware: Firmware, test_name: str): app_client = EthAppClient(backend) - if firmware.device not in ["stax", "flex"]: + if firmware.is_nano: pytest.skip("Not supported on non-NBGL apps") try: @@ -146,7 +145,6 @@ def test_blind_sign_reject_in_risk_review(firmware: Firmware, def test_sign_parameter_selector(firmware: Firmware, backend: BackendInterface, navigator: Navigator, - scenario_navigator: NavigateWithScenario, test_name: str, default_screenshot_path: Path): global DEVICE_ADDR @@ -168,8 +166,8 @@ def test_sign_parameter_selector(firmware: Firmware, flows += data_len // 32 with app_client.sign(BIP32_PATH, tx_params): moves = [] - if firmware.device.startswith("nano"): - if firmware.device == "nanos": + if firmware.is_nano: + if firmware == Firmware.NANOS: moves += [NavInsID.RIGHT_CLICK] * 2 + [NavInsID.BOTH_CLICK] # Parameters on Nano S are split on multiple pages, hardcoded because the two parameters don't use the # same amount of pages because of non-monospace fonts @@ -178,19 +176,19 @@ def test_sign_parameter_selector(firmware: Firmware, else: moves += ([NavInsID.RIGHT_CLICK] * 2 + [NavInsID.BOTH_CLICK]) * flows - if firmware.device == "nanos": + if firmware == Firmware.NANOS: moves += [NavInsID.RIGHT_CLICK] * 2 else: moves += [NavInsID.RIGHT_CLICK] * 4 moves += [NavInsID.BOTH_CLICK] - if firmware.device == "nanos": + if firmware == Firmware.NANOS: moves += [NavInsID.RIGHT_CLICK] * 9 else: moves += [NavInsID.RIGHT_CLICK] * 5 moves += [NavInsID.BOTH_CLICK] else: - if firmware.device == "stax": + if firmware == Firmware.STAX: tap_number = 2 else: tap_number = 3 diff --git a/tests/ragger/test_domain_name.py b/tests/ragger/test_domain_name.py index e0adc2c..96fa270 100644 --- a/tests/ragger/test_domain_name.py +++ b/tests/ragger/test_domain_name.py @@ -1,4 +1,3 @@ -from pathlib import Path import pytest from web3 import Web3 @@ -33,7 +32,7 @@ def verbose_fixture(request) -> bool: def common(firmware: Firmware, app_client: EthAppClient) -> int: - if firmware.device == "nanos": + if firmware == Firmware.NANOS: pytest.skip("Not supported on LNS") challenge = app_client.get_challenge() return ResponseParser.challenge(challenge.data) @@ -43,7 +42,6 @@ def test_send_fund(firmware: Firmware, backend: BackendInterface, navigator: Navigator, scenario_navigator: NavigateWithScenario, - default_screenshot_path: Path, verbose: bool): app_client = EthAppClient(backend) challenge = common(firmware, app_client) @@ -62,12 +60,12 @@ def test_send_fund(firmware: Firmware, "value": Web3.to_wei(AMOUNT, "ether"), "chainId": CHAIN_ID }): - if firmware.device.startswith("nano"): + if firmware.is_nano: end_text = "Accept" else: end_text = "Sign" - scenario_navigator.review_approve(default_screenshot_path, f"domain_name_verbose_{str(verbose)}", end_text) + scenario_navigator.review_approve(test_name=f"domain_name_verbose_{str(verbose)}", custom_screen_text=end_text) def test_send_fund_wrong_challenge(firmware: Firmware, backend: BackendInterface): @@ -81,8 +79,7 @@ def test_send_fund_wrong_challenge(firmware: Firmware, backend: BackendInterface def test_send_fund_wrong_addr(firmware: Firmware, backend: BackendInterface, - scenario_navigator: NavigateWithScenario, - default_screenshot_path: Path): + scenario_navigator: NavigateWithScenario): app_client = EthAppClient(backend) challenge = common(firmware, app_client) @@ -100,18 +97,17 @@ def test_send_fund_wrong_addr(firmware: Firmware, "value": Web3.to_wei(AMOUNT, "ether"), "chainId": CHAIN_ID }): - if firmware.device.startswith("nano"): + if firmware.is_nano: end_text = "Accept" else: end_text = "Sign" - scenario_navigator.review_approve(default_screenshot_path, "domain_name_wrong_addr", end_text) + scenario_navigator.review_approve(test_name="domain_name_wrong_addr", custom_screen_text=end_text) def test_send_fund_non_mainnet(firmware: Firmware, backend: BackendInterface, - scenario_navigator: NavigateWithScenario, - default_screenshot_path: Path): + scenario_navigator: NavigateWithScenario): app_client = EthAppClient(backend) challenge = common(firmware, app_client) @@ -126,18 +122,17 @@ def test_send_fund_non_mainnet(firmware: Firmware, "value": Web3.to_wei(AMOUNT, "ether"), "chainId": 5 }): - if firmware.device.startswith("nano"): + if firmware.is_nano: end_text = "Accept" else: end_text = "Sign" - scenario_navigator.review_approve(default_screenshot_path, "domain_name_non_mainnet", end_text) + scenario_navigator.review_approve(test_name="domain_name_non_mainnet", custom_screen_text=end_text) def test_send_fund_unknown_chain(firmware: Firmware, backend: BackendInterface, - scenario_navigator: NavigateWithScenario, - default_screenshot_path: Path): + scenario_navigator: NavigateWithScenario): app_client = EthAppClient(backend) challenge = common(firmware, app_client) @@ -152,12 +147,12 @@ def test_send_fund_unknown_chain(firmware: Firmware, "value": Web3.to_wei(AMOUNT, "ether"), "chainId": 9 }): - if firmware.device.startswith("nano"): + if firmware.is_nano: end_text = "Accept" else: end_text = "Sign" - scenario_navigator.review_approve(default_screenshot_path, "domain_name_unknown_chain", end_text) + scenario_navigator.review_approve(test_name="domain_name_unknown_chain", custom_screen_text=end_text) def test_send_fund_domain_too_long(firmware: Firmware, backend: BackendInterface): diff --git a/tests/ragger/test_eip191.py b/tests/ragger/test_eip191.py index 832bc85..1edd22e 100644 --- a/tests/ragger/test_eip191.py +++ b/tests/ragger/test_eip191.py @@ -1,4 +1,3 @@ -from pathlib import Path import pytest from ragger.error import ExceptionRAPDU @@ -17,7 +16,6 @@ BIP32_PATH = "m/44'/60'/0'/0/0" def common(backend: BackendInterface, scenario: NavigateWithScenario, test_name: str, - screenshot_path: Path, msg: str): app_client = EthAppClient(backend) @@ -27,7 +25,7 @@ def common(backend: BackendInterface, _, DEVICE_ADDR, _ = ResponseParser.pk_addr(app_client.response().data) with app_client.personal_sign(BIP32_PATH, msg.encode('utf-8')): - scenario.review_approve(screenshot_path, test_name, "Sign") + scenario.review_approve(test_name=test_name, custom_screen_text="Sign") # verify signature vrs = ResponseParser.signature(app_client.response().data) @@ -37,29 +35,26 @@ def common(backend: BackendInterface, def test_personal_sign_metamask(backend: BackendInterface, scenario_navigator: NavigateWithScenario, - test_name: str, - default_screenshot_path: Path): + test_name: str): msg = "Example `personal_sign` message" - common(backend, scenario_navigator, test_name, default_screenshot_path, msg) + common(backend, scenario_navigator, test_name, msg) def test_personal_sign_non_ascii(backend: BackendInterface, scenario_navigator: NavigateWithScenario, - test_name: str, - default_screenshot_path: Path): + test_name: str): msg = "0x9c22ff5f21f0b81b113e63f7db6da94fedef11b2119b4088b89664fb9a3cb658" - common(backend, scenario_navigator, test_name, default_screenshot_path, msg) + common(backend, scenario_navigator, test_name, msg) def test_personal_sign_opensea(firmware: Firmware, backend: BackendInterface, scenario_navigator: NavigateWithScenario, - test_name: str, - default_screenshot_path: Path): + test_name: str): - if firmware.device == "nanos": + if firmware == Firmware.NANOS: pytest.skip("Not supported on LNS") msg = "Welcome to OpenSea!\n\n" @@ -67,14 +62,12 @@ def test_personal_sign_opensea(firmware: Firmware, msg += "This request will not trigger a blockchain transaction or cost any gas fees.\n\n" msg += "Your authentication status will reset after 24 hours.\n\n" msg += "Wallet address:\n0x9858effd232b4033e47d90003d41ec34ecaeda94\n\nNonce:\n2b02c8a0-f74f-4554-9821-a28054dc9121" - common(backend, scenario_navigator, test_name, default_screenshot_path, msg) + common(backend, scenario_navigator, test_name, msg) def test_personal_sign_reject(firmware: Firmware, backend: BackendInterface, - scenario_navigator: NavigateWithScenario, - test_name: str, - default_screenshot_path: Path): + scenario_navigator: NavigateWithScenario): msg = "This is an reject sign" @@ -82,11 +75,11 @@ def test_personal_sign_reject(firmware: Firmware, try: with app_client.personal_sign(BIP32_PATH, msg.encode('utf-8')): - if firmware.device.startswith("nano"): + if firmware.is_nano: end_text = "Cancel" else: end_text = "Sign" - scenario_navigator.review_reject(default_screenshot_path, test_name, end_text) + scenario_navigator.review_reject(custom_screen_text=end_text) except ExceptionRAPDU as e: assert e.status == StatusWord.CONDITION_NOT_SATISFIED diff --git a/tests/ragger/test_eip712.py b/tests/ragger/test_eip712.py index 4844d0d..1f64fb1 100644 --- a/tests/ragger/test_eip712.py +++ b/tests/ragger/test_eip712.py @@ -90,7 +90,7 @@ def test_eip712_legacy(backend: BackendInterface, scenario_navigator: NavigateWi def autonext(firmware: Firmware, navigator: Navigator, default_screenshot_path: Path): moves = [] - if firmware.device.startswith("nano"): + if firmware.is_nano: moves = [NavInsID.RIGHT_CLICK] else: moves = [NavInsID.SWIPE_CENTER_TO_LEFT] @@ -123,10 +123,10 @@ def eip712_new_common(firmware: Firmware, golden_run) with app_client.eip712_sign_new(BIP32_PATH): moves = [] - if firmware.device.startswith("nano"): + if firmware.is_nano: # need to skip the message hash if not verbose and filters is None: - moves = [NavInsID.RIGHT_CLICK] * 2 + moves += [NavInsID.RIGHT_CLICK] * 2 moves += [NavInsID.BOTH_CLICK] else: # this move is necessary most of the times, but can't be 100% sure with the fields grouping @@ -159,7 +159,7 @@ def test_eip712_new(firmware: Firmware, verbose: bool, filtering: bool): app_client = EthAppClient(backend) - if firmware.device == "nanos": + if firmware == Firmware.NANOS: pytest.skip("Not supported on LNS") test_path = f"{input_file.parent}/{'-'.join(input_file.stem.split('-')[:-1])}" @@ -420,7 +420,7 @@ def test_eip712_advanced_filtering(firmware: Firmware, global SNAPS_CONFIG app_client = EthAppClient(backend) - if firmware.device == "nanos": + if firmware == Firmware.NANOS: pytest.skip("Not supported on LNS") SNAPS_CONFIG = SnapshotsConfig(test_name + data_set.suffix) diff --git a/tests/ragger/test_get_address.py b/tests/ragger/test_get_address.py index a9bef71..035c598 100644 --- a/tests/ragger/test_get_address.py +++ b/tests/ragger/test_get_address.py @@ -1,4 +1,3 @@ -from pathlib import Path from typing import Optional import pytest @@ -36,27 +35,25 @@ def chain_fixture(request) -> Optional[int]: ) def test_get_pk_rejected(backend: BackendInterface, scenario_navigator: NavigateWithScenario, - default_screenshot_path: Path, path, suffix): app_client = EthAppClient(backend) with pytest.raises(ExceptionRAPDU) as e: with app_client.get_public_addr(bip32_path=path): - scenario_navigator.address_review_reject(default_screenshot_path, f"get_pk_rejected_{suffix}") + scenario_navigator.address_review_reject(test_name=f"get_pk_rejected_{suffix}") assert e.value.status == StatusWord.CONDITION_NOT_SATISFIED def test_get_pk(backend: BackendInterface, - default_screenshot_path: Path, scenario_navigator: NavigateWithScenario, with_chaincode: bool, chain: Optional[int]): app_client = EthAppClient(backend) with app_client.get_public_addr(chaincode=with_chaincode, chain_id=chain): - scenario_navigator.address_review_approve(default_screenshot_path, f"get_pk_{chain}") + scenario_navigator.address_review_approve(test_name=f"get_pk_{chain}") pk, _, chaincode = ResponseParser.pk_addr(app_client.response().data, with_chaincode) ref_pk, ref_chaincode = calculate_public_key_and_chaincode(curve=CurveChoice.Secp256k1, @@ -69,18 +66,17 @@ def test_get_pk(backend: BackendInterface, def test_get_eth2_pk(firmware: Firmware, backend: BackendInterface, scenario_navigator: NavigateWithScenario, - test_name: str, - default_screenshot_path: Path): + test_name: str): app_client = EthAppClient(backend) path="m/12381/3600/0/0" with app_client.get_eth2_public_addr(bip32_path=path): - scenario_navigator.address_review_approve(default_screenshot_path, test_name) + scenario_navigator.address_review_approve(test_name=test_name) pk = app_client.response().data ref_pk = bls.SkToPk(mnemonic_and_path_to_key(SPECULOS_MNEMONIC, path)) - if firmware.name in ("stax", "flex"): + if firmware in (Firmware.STAX, Firmware.FLEX): pk = pk[1:49] assert pk == ref_pk diff --git a/tests/ragger/test_nft.py b/tests/ragger/test_nft.py index abba24c..38ffa62 100644 --- a/tests/ragger/test_nft.py +++ b/tests/ragger/test_nft.py @@ -1,4 +1,3 @@ -from pathlib import Path from typing import Callable, Optional, Any import json import pytest @@ -51,7 +50,6 @@ class Action: def common_test_nft(firmware: Firmware, backend: BackendInterface, scenario_navigator: NavigateWithScenario, - default_screenshot_path: Path, collec: NFTCollection, action: Action, reject: bool, @@ -59,7 +57,7 @@ def common_test_nft(firmware: Firmware, global DEVICE_ADDR app_client = EthAppClient(backend) - if firmware.device == "nanos": + if firmware == Firmware.NANOS: pytest.skip("Not supported on LNS") if DEVICE_ADDR is None: # to only have to request it once @@ -86,13 +84,13 @@ def common_test_nft(firmware: Firmware, test_name = f"{plugin_name.lower()}_{action.fn_name}_{str(collec.chain_id)}" if reject: test_name += "-rejected" - scenario_navigator.review_reject(default_screenshot_path, test_name) + scenario_navigator.review_reject(test_name=test_name) else: - if firmware.device.startswith("nano"): + if firmware.is_nano: end_text = "Accept" else: end_text = "Sign" - scenario_navigator.review_approve(default_screenshot_path, test_name, end_text) + scenario_navigator.review_approve(test_name=test_name, custom_screen_text=end_text) # verify signature vrs = ResponseParser.signature(app_client.response().data) @@ -104,11 +102,10 @@ def common_test_nft_reject(test_fn: Callable, firmware: Firmware, backend: BackendInterface, scenario_navigator: NavigateWithScenario, - default_screenshot_path: Path, collec: NFTCollection, action: Action): with pytest.raises(ExceptionRAPDU) as e: - test_fn(firmware, backend, scenario_navigator, default_screenshot_path, collec, action, True) + test_fn(firmware, backend, scenario_navigator, collec, action, True) assert e.value.status == StatusWord.CONDITION_NOT_SATISFIED # ERC-721 @@ -158,14 +155,12 @@ def action_721_fixture(request) -> Action: def test_erc721(firmware: Firmware, backend: BackendInterface, scenario_navigator: NavigateWithScenario, - default_screenshot_path: Path, collec_721: NFTCollection, action_721: Action, reject: bool = False): common_test_nft(firmware, backend, scenario_navigator, - default_screenshot_path, collec_721, action_721, reject, @@ -174,13 +169,11 @@ def test_erc721(firmware: Firmware, def test_erc721_reject(firmware: Firmware, backend: BackendInterface, - scenario_navigator: NavigateWithScenario, - default_screenshot_path: Path): + scenario_navigator: NavigateWithScenario): common_test_nft_reject(test_erc721, firmware, backend, scenario_navigator, - default_screenshot_path, collecs_721[0], actions_721[0]) @@ -237,14 +230,12 @@ def action_1155_fixture(request) -> Action: def test_erc1155(firmware: Firmware, backend: BackendInterface, scenario_navigator: NavigateWithScenario, - default_screenshot_path: Path, collec_1155: NFTCollection, action_1155: Action, reject: bool = False): common_test_nft(firmware, backend, scenario_navigator, - default_screenshot_path, collec_1155, action_1155, reject, @@ -253,12 +244,10 @@ def test_erc1155(firmware: Firmware, def test_erc1155_reject(firmware: Firmware, backend: BackendInterface, - scenario_navigator: NavigateWithScenario, - default_screenshot_path: Path): + scenario_navigator: NavigateWithScenario): common_test_nft_reject(test_erc1155, firmware, backend, scenario_navigator, - default_screenshot_path, collecs_1155[0], actions_1155[0]) diff --git a/tests/ragger/test_sign.py b/tests/ragger/test_sign.py index b86cbf1..f7b720b 100644 --- a/tests/ragger/test_sign.py +++ b/tests/ragger/test_sign.py @@ -46,18 +46,18 @@ def common(firmware: Firmware, _, DEVICE_ADDR, _ = ResponseParser.pk_addr(app_client.response().data) with app_client.sign(path, tx_params): - if not firmware.device.startswith("nano") and confirm: + if not firmware.is_nano and confirm: navigator.navigate_and_compare(default_screenshot_path, f"{test_name}/confirm", [NavInsID.USE_CASE_CHOICE_CONFIRM], screen_change_after_last_instruction=False) - if firmware.device.startswith("nano"): + if firmware.is_nano: end_text = "Accept" else: end_text = "Sign" - scenario_navigator.review_approve(default_screenshot_path, test_name, end_text, (test_name != "")) + scenario_navigator.review_approve(custom_screen_text=end_text, do_comparison=test_name!="") # verify signature vrs = ResponseParser.signature(app_client.response().data) @@ -67,15 +67,13 @@ def common(firmware: Firmware, def common_reject(backend: BackendInterface, scenario_navigator: NavigateWithScenario, - default_screenshot_path: Path, tx_params: dict, - test_name: str, path: str = BIP32_PATH): app_client = EthAppClient(backend) try: with app_client.sign(path, tx_params): - scenario_navigator.review_reject(default_screenshot_path, test_name) + scenario_navigator.review_reject() except ExceptionRAPDU as e: assert e.status == StatusWord.CONDITION_NOT_SATISFIED @@ -235,10 +233,7 @@ def test_sign_nonce_display(firmware: Firmware, common(firmware, backend, navigator, scenario_navigator, default_screenshot_path, tx_params, test_name, "m/44'/60'/1'/0/0") -def test_sign_reject(backend: BackendInterface, - scenario_navigator: NavigateWithScenario, - test_name: str, - default_screenshot_path: Path): +def test_sign_reject(backend: BackendInterface, scenario_navigator: NavigateWithScenario): tx_params: dict = { "nonce": NONCE2, "gasPrice": Web3.to_wei(GAS_PRICE, 'gwei'), @@ -247,7 +242,7 @@ def test_sign_reject(backend: BackendInterface, "value": Web3.to_wei(AMOUNT2, "ether"), "chainId": CHAIN_ID } - common_reject(backend, scenario_navigator, default_screenshot_path, tx_params, test_name, "m/44'/60'/1'/0/0") + common_reject(backend, scenario_navigator, tx_params, "m/44'/60'/1'/0/0") def test_sign_error_transaction_type(backend: BackendInterface):