From 57582c2501faa23b4d265cdde8da4b9f0472f68d Mon Sep 17 00:00:00 2001 From: matejcik Date: Tue, 22 Jun 2021 11:01:29 +0200 Subject: [PATCH] feat(tests): simple javascript-based UI diff review tool --- docs/tests/ui-tests.md | 11 ++- tests/conftest.py | 2 +- tests/show_results.py | 71 +++++++++++++ tests/ui_tests/reporting/html.py | 7 +- tests/ui_tests/reporting/testreport.css | 53 ++++++++++ tests/ui_tests/reporting/testreport.js | 126 ++++++++++++++++++++++++ tests/ui_tests/reporting/testreport.py | 60 ++++++++++- 7 files changed, 319 insertions(+), 11 deletions(-) create mode 100755 tests/show_results.py create mode 100644 tests/ui_tests/reporting/testreport.css create mode 100644 tests/ui_tests/reporting/testreport.js diff --git a/docs/tests/ui-tests.md b/docs/tests/ui-tests.md index 658ba665c..54920b45a 100644 --- a/docs/tests/ui-tests.md +++ b/docs/tests/ui-tests.md @@ -79,10 +79,15 @@ pytest tests/device_tests --ui=record --ui-check-missing ### Tests Each `--ui=test` creates a clear report which tests passed and which failed. -The index file is stored in `tests/ui_tests/reporting/reports/test/index.html`, but for an ease of use -you will find a link at the end of the pytest summary. +The index file is stored in `tests/ui_tests/reporting/reports/test/index.html`. +The script `tests/show_results.py` starts a local HTTP server that serves this page -- +this is necessary for access to browser local storage, which enables a simple reviewer +UI. -On CI this report is published as an artifact. You can see the latest master report [here](https://gitlab.com/satoshilabs/trezor/trezor-firmware/-/jobs/artifacts/master/file/test_ui_report/index.html?job=core%20device%20ui%20test). +On CI this report is published as an artifact. You can see the latest master report [here](https://gitlab.com/satoshilabs/trezor/trezor-firmware/-/jobs/artifacts/master/file/test_ui_report/index.html?job=core%20device%20ui%20test). The reviewer features work directly here. + +If needed, you can use `python3 -m tests.ui_tests` to regenerate the report from local +recorded screens. ### Master diff diff --git a/tests/conftest.py b/tests/conftest.py index afcbe1c41..8e3536a65 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -202,7 +202,7 @@ def pytest_terminal_summary(terminalreporter, exitstatus, config): if _should_write_ui_report(exitstatus): println("-------- UI tests summary: --------") - println(f"{testreport.REPORTS_PATH / 'index.html'}") + println("Run ./tests/show_results.py to open test summary") println("") diff --git a/tests/show_results.py b/tests/show_results.py new file mode 100755 index 000000000..1b60b58fd --- /dev/null +++ b/tests/show_results.py @@ -0,0 +1,71 @@ +#!/usr/bin/env python3 + +import http.server +import multiprocessing +import os +import posixpath +import time +import urllib +import webbrowser +from pathlib import Path + +import click + +ROOT = Path(__file__).parent.parent.resolve() +TEST_RESULT_PATH = ROOT / "tests" / "ui_tests" / "reporting" / "reports" / "test" + + +class NoCacheRequestHandler(http.server.SimpleHTTPRequestHandler): + def end_headers(self): + self.send_header("Cache-Control", "no-cache, no-store, must-revalidate") + self.send_header("Pragma", "no-cache") + self.send_header("Expires", "0") + return super().end_headers() + + def log_message(self, format, *args): + pass + + def translate_path(self, path): + # XXX + # Copy-pasted from Python 3.8 BaseHTTPRequestHandler so that we can inject + # the `directory` parameter. + # Otherwise, to keep compatible with 3.6, we'd need to mess with CWD. Which is + # unstable when we expect it to be erased and recreated under us. + path = path.split("?", 1)[0] + path = path.split("#", 1)[0] + # Don't forget explicit trailing slash when normalizing. Issue17324 + trailing_slash = path.rstrip().endswith("/") + try: + path = urllib.parse.unquote(path, errors="surrogatepass") + except UnicodeDecodeError: + path = urllib.parse.unquote(path) + path = posixpath.normpath(path) + words = path.split("/") + words = filter(None, words) + path = str(TEST_RESULT_PATH) # XXX this is the only modified line + for word in words: + if os.path.dirname(word) or word in (os.curdir, os.pardir): + # Ignore components that are not a simple file/directory name + continue + path = os.path.join(path, word) + if trailing_slash: + path += "/" + return path + + +def launch_http_server(port): + http.server.test(HandlerClass=NoCacheRequestHandler, bind="localhost", port=port) + + +@click.command() +@click.option("-p", "--port", type=int, default=8000) +def main(port): + httpd = multiprocessing.Process(target=launch_http_server, args=(port,)) + httpd.start() + time.sleep(0.5) + webbrowser.open(f"http://localhost:{port}/") + httpd.join() + + +if __name__ == "__main__": + main() diff --git a/tests/ui_tests/reporting/html.py b/tests/ui_tests/reporting/html.py index 8f793ef70..c4bbeef6c 100644 --- a/tests/ui_tests/reporting/html.py +++ b/tests/ui_tests/reporting/html.py @@ -5,7 +5,10 @@ from itertools import zip_longest from dominate.tags import a, i, img, table, td, th, tr -def report_links(tests, reports_path): +def report_links(tests, reports_path, actual_hashes=None): + if actual_hashes is None: + actual_hashes = {} + if not tests: i("None!") return @@ -13,7 +16,7 @@ def report_links(tests, reports_path): with tr(): th("Link to report") for test in sorted(tests): - with tr(): + with tr(data_actual_hash=actual_hashes.get(test.stem, "")): path = test.relative_to(reports_path) td(a(test.name, href=path)) diff --git a/tests/ui_tests/reporting/testreport.css b/tests/ui_tests/reporting/testreport.css new file mode 100644 index 000000000..f526d65ab --- /dev/null +++ b/tests/ui_tests/reporting/testreport.css @@ -0,0 +1,53 @@ +.novisit a:visited { + color: blue; +} + +tr.ok a, tr.ok a:visited { + color: grey; +} + +tr.bad a, tr.bad a:visited { + color: darkred; +} + +#markbox { + position: fixed; + top: 50px; + right: 5px; + width: 300px; + padding: 1em; +} + +#markbox #buttons { + display: flex; + justify-content: space-evenly; +} + +#markbox button { + border: 3px solid; + font-size: 20pt; + padding: 1em; + background: white; +} + +#markbox #mark-ok { + color: green; + border-color: darkgreen; +} + +#markbox #mark-ok:hover { + border-color: lightgreen; +} + +#markbox #mark-bad { + color: darkred; + border-color: darkred; +} + +#markbox #mark-bad:hover { + border-color: red; +} + +.script-hidden { + display: none; +} diff --git a/tests/ui_tests/reporting/testreport.js b/tests/ui_tests/reporting/testreport.js new file mode 100644 index 000000000..98b938f87 --- /dev/null +++ b/tests/ui_tests/reporting/testreport.js @@ -0,0 +1,126 @@ + + +function refreshMarkStates() { + for (let tr of document.body.querySelectorAll("tr[data-actual-hash]")) { + let a = tr.querySelector("a") + let mark = window.localStorage.getItem(a.href) + tr.className = mark || "" + } +} + + +function markState(state) { + window.localStorage.setItem(window.location.href, state) + if (window.nextHref) { + window.location.assign(window.nextHref) + } else { + window.close() + } +} + + +function resetState(whichState) { + function shouldReset(value) { + if (value == whichState) return true + if (whichState != "all") return false + return (value == "bad" || value == "ok") + } + + let keysToReset = [] + + for (let i = 0; i < window.localStorage.length; ++i) { + let key = window.localStorage.key(i) + let value = window.localStorage.getItem(key) + if (shouldReset(value)) keysToReset.push(key) + } + + for (let key of keysToReset) { + window.localStorage.removeItem(key) + } + + refreshMarkStates() +} + + +function findNextForHref(doc, href) { + let foundIt = false; + for (let tr of doc.body.querySelectorAll("tr")) { + if (!tr.dataset.actualHash) continue + let a = tr.querySelector("a") + if (!a) continue + if (foundIt) return a.href + else if (a.href == href) foundIt = true + } +} + + +function openLink(ev) { + if (ev.button == 2) { + // let right click through + return true; + } + + // capture other clicks + ev.preventDefault() + let href = ev.target.href + let newWindow = window.open(href) + newWindow +} + + +function onLoadIndex() { + document.getElementById("file-hint").hidden = true + + for (let a of document.body.querySelectorAll("a[href]")) { + a.onclick = openLink + a.onauxclick = openLink + } + + document.body.classList.add("novisit") + + window.onstorage = refreshMarkStates + refreshMarkStates() +} + + +function onLoadTestCase() { + if (window.opener) { + window.nextHref = findNextForHref(window.opener.document, window.location.href) + if (window.nextHref) { + markbox = document.getElementById("markbox") + par = document.createElement("p") + par.append("and proceed to ") + a = document.createElement("a") + a.append("next case") + a.href = window.nextHref + a.onclick = ev => { + console.log("on click") + ev.preventDefault() + window.location.assign(window.nextHref) + } + + par.append(a) + markbox.append(par) + } + } else { + window.nextHref = null + } +} + + +function onLoad() { + if (window.location.protocol == "file") return + + for (let elem of document.getElementsByClassName("script-hidden")) { + elem.classList.remove("script-hidden") + } + + if (document.body.dataset.index) { + onLoadIndex() + } else { + onLoadTestCase() + } +} + + +window.onload = onLoad diff --git a/tests/ui_tests/reporting/testreport.py b/tests/ui_tests/reporting/testreport.py index f57027b61..a3b6b24ea 100644 --- a/tests/ui_tests/reporting/testreport.py +++ b/tests/ui_tests/reporting/testreport.py @@ -4,12 +4,36 @@ from distutils.dir_util import copy_tree from pathlib import Path import dominate +import dominate.tags as t from dominate.tags import div, h1, h2, hr, p, strong, table, th, tr from dominate.util import text from . import download, html -REPORTS_PATH = Path(__file__).parent.resolve() / "reports" / "test" +HERE = Path(__file__).parent.resolve() +REPORTS_PATH = HERE / "reports" / "test" + +STYLE = (HERE / "testreport.css").read_text() +SCRIPT = (HERE / "testreport.js").read_text() + +ACTUAL_HASHES = {} + + +def document(title, actual_hash=None, index=False): + doc = dominate.document(title=title) + style = t.style() + style.add_raw_string(STYLE) + script = t.script() + script.add_raw_string(SCRIPT) + doc.head.add(style, script) + + if actual_hash is not None: + doc.body["data-actual-hash"] = actual_hash + + if index: + doc.body["data-index"] = True + + return doc def _header(test_name, expected_hash, actual_hash): @@ -43,7 +67,7 @@ def index(): failed_tests = list((REPORTS_PATH / "failed").iterdir()) title = "UI Test report " + datetime.now().strftime("%Y-%m-%d %H:%M:%S") - doc = dominate.document(title=title) + doc = document(title=title, index=True) with doc: h1("UI Test report") @@ -54,7 +78,25 @@ def index(): hr() h2("Failed", style="color: red;") - html.report_links(failed_tests, REPORTS_PATH) + with p(id="file-hint"): + strong("Tip:") + text(" use ") + t.span("./tests/show_results.sh", style="font-family: monospace") + text(" to enable smart features.") + + with div("Test colors", _class="script-hidden"): + with t.ul(): + with t.li(): + t.span("new", style="color: blue") + t.button("clear all", onclick="resetState('all')") + with t.li(): + t.span("marked OK", style="color: grey") + t.button("clear", onclick="resetState('ok')") + with t.li(): + t.span("marked BAD", style="color: darkred") + t.button("clear", onclick="resetState('bad')") + + html.report_links(failed_tests, REPORTS_PATH, ACTUAL_HASHES) h2("Passed", style="color: green;") html.report_links(passed_tests, REPORTS_PATH) @@ -63,7 +105,9 @@ def index(): def failed(fixture_test_path, test_name, actual_hash, expected_hash): - doc = dominate.document(title=test_name) + ACTUAL_HASHES[test_name] = actual_hash + + doc = document(title=test_name, actual_hash=actual_hash) recorded_path = fixture_test_path / "recorded" actual_path = fixture_test_path / "actual" @@ -82,6 +126,12 @@ def failed(fixture_test_path, test_name, actual_hash, expected_hash): with doc: _header(test_name, expected_hash, actual_hash) + with div(id="markbox", _class="script-hidden"): + p("Click a button to mark the test result as:") + with div(id="buttons"): + t.button("OK", id="mark-ok", onclick="markState('ok')") + t.button("BAD", id="mark-bad", onclick="markState('bad')") + if download_failed: with p(): strong("WARNING:") @@ -100,7 +150,7 @@ def failed(fixture_test_path, test_name, actual_hash, expected_hash): def passed(fixture_test_path, test_name, actual_hash): copy_tree(str(fixture_test_path / "actual"), str(fixture_test_path / "recorded")) - doc = dominate.document(title=test_name) + doc = document(title=test_name) actual_path = fixture_test_path / "actual" actual_screens = sorted(actual_path.iterdir())