From 56257eb6a53bced3ac51f173d50ff8105c192963 Mon Sep 17 00:00:00 2001 From: Tomas Susanka Date: Mon, 6 Jan 2020 14:44:30 +0000 Subject: [PATCH] tests/ui: code review fixes --- ci/prepare_ui_artifacts.py | 27 +++++++++---------------- core/Makefile | 4 ++-- tests/conftest.py | 2 +- tests/ui_tests/__init__.py | 40 +++++++++++++------------------------- tests/ui_tests/html.py | 5 +---- 5 files changed, 27 insertions(+), 51 deletions(-) diff --git a/ci/prepare_ui_artifacts.py b/ci/prepare_ui_artifacts.py index d18be4bb74..b543bfe642 100644 --- a/ci/prepare_ui_artifacts.py +++ b/ci/prepare_ui_artifacts.py @@ -3,28 +3,19 @@ import shutil from pathlib import Path -def _hash_files(files): +def _hash_files(path): + files = path.iterdir() hasher = hashlib.sha256() for file in sorted(files): - with open(file, "rb") as f: - content = f.read() - hasher.update(content) + hasher.update(file.read_bytes()) return hasher.digest().hex() -def _compare_hash(test_dir, hash): - with open(test_dir / "hash.txt", "r") as f: - content = f.read() - assert hash == content +fixture_root = Path().cwd() / "../tests/ui_tests/fixtures/" - -fixture_root = Path().cwd() / "../tests/ui_tests/fixtures" - -for test_dir in fixture_root.iterdir(): - if test_dir.is_dir(): - recorded_dir = test_dir / "recorded" - if recorded_dir.exists(): - hash = _hash_files(recorded_dir.iterdir()) - _compare_hash(test_dir, hash) - shutil.make_archive("tmp/" + hash, "zip", recorded_dir) +for recorded_dir in fixture_root.glob("*/recorded"): + expected_hash = (recorded_dir.parent / "hash.txt").read_text() + actual_hash = _hash_files(recorded_dir) + assert expected_hash == actual_hash + shutil.make_archive("tmp/" + actual_hash, "zip", recorded_dir) diff --git a/core/Makefile b/core/Makefile index 6e21e071b5..734993940f 100644 --- a/core/Makefile +++ b/core/Makefile @@ -80,10 +80,10 @@ test_emu_click: ## run click tests cd tests ; ./run_tests_click_emu.sh $(TESTOPTS) test_emu_ui: ## run ui integration tests - cd tests ; ./run_tests_device_emu.sh --test_screen=test -m "not skip_ui" $(TESTOPTS) + cd tests ; ./run_tests_device_emu.sh --test-screen=test -m "not skip_ui" $(TESTOPTS) test_emu_ui_record: ## record and hash screens for ui integration tests - cd tests ; ./run_tests_device_emu.sh --test_screen=record -m "not skip_ui" $(TESTOPTS) + cd tests ; ./run_tests_device_emu.sh --test-screen=record -m "not skip_ui" $(TESTOPTS) pylint: ## run pylint on application sources and tests pylint -E $(shell find src tests -name *.py) diff --git a/tests/conftest.py b/tests/conftest.py index 80bd2c3221..eab6a66853 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -140,7 +140,7 @@ def client(request): def pytest_addoption(parser): parser.addoption( - "--test_screen", + "--test-screen", action="store", default="", help="Enable UI intergration tests: 'record' or 'test'", diff --git a/tests/ui_tests/__init__.py b/tests/ui_tests/__init__.py index 9c51149ba0..a989adc5e8 100644 --- a/tests/ui_tests/__init__.py +++ b/tests/ui_tests/__init__.py @@ -18,7 +18,7 @@ def _get_test_dirname(node): # we limit the name to first 100 chars. This is not a problem with txhashes. node_name = re.sub(r"\W+", "_", node.name)[:100] node_module_name = node.getparent(pytest.Module).name - return "{}_{}".format(node_module_name, node_name) + return f"{node_module_name}_{node_name}" def _check_fixture_directory(fixture_dir, screen_path): @@ -32,28 +32,24 @@ def _check_fixture_directory(fixture_dir, screen_path): def _process_recorded(screen_path): - records = sorted(screen_path.iterdir()) - # create hash - digest = _hash_files(records) - with open(screen_path / "../hash.txt", "w") as f: - f.write(digest) + digest = _hash_files(screen_path) + + (screen_path.parent / "hash.txt").write_text(digest) _rename_records(screen_path) def _rename_records(screen_path): # rename screenshots for index, record in enumerate(sorted(screen_path.iterdir())): - filename = screen_path / "{:08}.png".format(index) - record.replace(filename) + record.replace(screen_path / f"{index:08}.png") -def _hash_files(files): +def _hash_files(path): + files = path.iterdir() hasher = hashlib.sha256() for file in sorted(files): - with open(file, "rb") as f: - content = f.read() - hasher.update(content) + hasher.update(file.read_bytes()) return hasher.digest().hex() @@ -64,14 +60,11 @@ def _process_tested(fixture_test_path, test_name): if not hash_file.exists(): raise ValueError("File hash.txt not found.") - with open(hash_file, "r") as f: - expected_hash = f.read() - + expected_hash = hash_file.read_text() actual_path = fixture_test_path / "actual" - _rename_records(actual_path) + actual_hash = _hash_files(actual_path) - records = sorted(actual_path.iterdir()) - actual_hash = _hash_files(records) + _rename_records(actual_path) if actual_hash != expected_hash: diff_file = html.diff_file( @@ -93,18 +86,13 @@ def _process_tested(fixture_test_path, test_name): @contextmanager def screen_recording(client, request): - if not request.node.get_closest_marker("skip_ui"): - test_screen = request.config.getoption("test_screen") - else: - test_screen = "" - - if not test_screen: + test_screen = request.config.getoption("test_screen") + if request.node.get_closest_marker("skip_ui") or not test_screen: yield return - fixture_root = Path(__file__) / "../fixtures" test_name = _get_test_dirname(request.node) - fixture_test_path = fixture_root.resolve() / test_name + fixture_test_path = Path(__file__).parent.resolve() / "fixtures" / test_name if test_screen == "record": screen_path = fixture_test_path / "recorded" diff --git a/tests/ui_tests/html.py b/tests/ui_tests/html.py index 29d39eab37..95c4ec5c37 100644 --- a/tests/ui_tests/html.py +++ b/tests/ui_tests/html.py @@ -61,8 +61,5 @@ def diff_file(fixture_test_path, test_name, actual_hash, expected_hash): _image(r) _image(a) - with open(fixture_test_path / "diff.html", "w") as f: - f.write(doc.render()) - f.close() - + (fixture_test_path / "diff.html").write_text(doc.render()) return fixture_test_path / "diff.html"