feat(tests): simple javascript-based UI diff review tool

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
+The index file is stored in `tests/ui_tests/reporting/reports/test/index.html`.
-you will find a link at the end of the pytest summary.
+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
 

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("")
 
 

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()

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))
 

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;
+}

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

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())