functionaltests: adapt compare test to new replayer

v2:
  - Respect the 3 possible return values when invoking
    compare.compare.

v3:
  - Do not use "len()" to check for containers emptiness (Dylan).
  - Use newly added compare result enumeration.
  - Add extension check to the testtrace backend.

v4:
  - Return directly from within the context manager (Dylan).
  - Use os.path.join instead of just concatenating strings (Dylan).
  - Use textwrap.dedent to enhance readability (Dylan).

Signed-off-by: Andres Gomez <agomez@igalia.com>
Reviewed-by: Dylan Baker <dylan@pnwbakers.com>
Part-of: <https://gitlab.freedesktop.org/mesa/piglit/-/merge_requests/353>

2 files changed, 114 insertions, 71 deletions

diff --git a/functionaltests/framework/replay/backends/testtrace.py b/functionaltests/framework/replay/programs/backends/testtrace.py
index 08f8b717e..6c5700255 100644
--- a/functionaltests/framework/replay/backends/testtrace.py
+++ b/functionaltests/framework/replay/programs/backends/testtrace.py
@@ -32,9 +32,9 @@ import subprocess
 
 from os import path
 
-from framework import core
-from .abstract import DumpBackend, dump_handler
-from .register import Registry
+from framework import core, exceptions
+from framework.replay.backends.abstract import DumpBackend, dump_handler
+from framework.replay.backends.register import Registry
 
 
 __all__ = [
@@ -52,10 +52,18 @@ class TestTraceBackend(DumpBackend):
     """
     _get_last_frame_call = None  # this silences the abstract-not-subclassed warning
 
-    def __init__(self, trace_path, output_dir=None, calls=[], **kwargs):
+    def __init__(self, trace_path, output_dir=None, calls=None, **kwargs):
         super(TestTraceBackend, self).__init__(trace_path, output_dir, calls,
                                                **kwargs)
-        if len(self._calls) == 0: self._calls = ['0']
+        extension = path.splitext(self._trace_path)[1]
+
+        if extension != '.testtrace':
+            raise exceptions.PiglitFatalError(
+                'Invalid trace_path: "{}" tried to be dumped '
+                'by the TestTraceBackend.\n'.format(self._trace_path))
+
+        if not self._calls:
+            self._calls = ['0']
 
     @dump_handler
     def dump(self):
diff --git a/functionaltests/framework/replay/programs/test_compare.py b/functionaltests/framework/replay/programs/test_compare.py
index d2f8cec7e..21d6b886c 100644
--- a/functionaltests/framework/replay/programs/test_compare.py
+++ b/functionaltests/framework/replay/programs/test_compare.py
@@ -1,24 +1,53 @@
+# coding=utf-8
+#
+# Copyright (c) 2019 Collabora Ltd
+# Copyright © 2019-2020 Valve Corporation.
+#
+# Permission is hereby granted, free of charge, to any person obtaining a
+# copy of this software and associated documentation files (the "Software"),
+# to deal in the Software without restriction, including without limitation
+# the rights to use, copy, modify, merge, publish, distribute, sublicense,
+# and/or sell copies of the Software, and to permit persons to whom the
+# Software is furnished to do so, subject to the following conditions:
+#
+# The above copyright notice and this permission notice shall be included
+# in all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+# OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
+# OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+# ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+# OTHER DEALINGS IN THE SOFTWARE.
+#
+# SPDX-License-Identifier: MIT
+
 import logging
+import contextlib
 import os
 import pytest
 import re
 import shutil
+import textwrap
 
 from os import path
 
-import tracie
+from framework.replay import backends
+from framework.replay.programs import compare
+from framework.replay.compare_replay import Result
+
+from .backends import testtrace
 
 
-RESULTS_YAML = "results/results.yml"
-TRACE_LOG_TEST1 = "results/trace1/test/gl-test-device/magenta.testtrace.log"
-TRACE_LOG_TEST2 = "results/trace2/test/vk-test-device/olive.testtrace.log"
-TRACE_PNG_TEST1 = "results/trace1/test/gl-test-device/magenta.testtrace-0.png"
-TRACE_PNG_TEST2 = "results/trace2/test/vk-test-device/olive.testtrace-0.png"
-TRACIE_DIR = path.dirname(path.realpath(__file__)) + "/.."
+TESTS_OUTPUT = "results/output.txt"
+TRACE_PNG_TEST1 = "results/trace/gl-test-device/trace1/magenta.testtrace-0.png"
+TRACE_PNG_TEST2 = "results/trace/vk-test-device/trace2/olive.testtrace-0.png"
 
 logging.basicConfig(level=logging.INFO)
 logger = logging.getLogger()
 
+backends.DUMPBACKENDS["testtrace"] = testtrace.REGISTRY
 
 def write_to(content, filename):
     with open(filename, 'w') as f:
@@ -28,38 +57,44 @@ def write_to(content, filename):
 def read_from(filename):
     with open(filename) as f:
         content = f.read()
-    return content
+        return content
 
 
-def run_tracie():
+def run_compare(extra_args):
     '''
     Run tests for the .testtrace types, using the "gl-test-device" and
     "vk-test-device" device names.
     '''
-    result = tracie.main(["yaml",
-                          "--device-name", "gl-test-device",
-                          "--file", "./tests/traces.yml"])
-    if not result:
-        return False
-    result = tracie.main(["yaml",
-                          "--device-name", "vk-test-device",
-                          "--file", "./tests/traces.yml"])
+    result = 0
+    os.makedirs(path.dirname(TESTS_OUTPUT), exist_ok=True)
+    with open(TESTS_OUTPUT, 'w') as f:
+        with contextlib.redirect_stdout(f):
+            result = compare.compare(["yaml",
+                                      "--device-name", "gl-test-device",
+                                      "--yaml-file", "./tests/traces.yml"] + extra_args)
+    if result is not Result.MATCH:
+        return result
+    with open(TESTS_OUTPUT, 'a') as f:
+        with contextlib.redirect_stdout(f):
+            result = compare.compare(["yaml",
+                                      "--device-name", "vk-test-device",
+                                      "--yaml-file", "./tests/traces.yml"] + extra_args)
     return result
 
 
 def prepare_for_run(tmp_path):
     '''
-    Copy all the tracie scripts to the test dir for the unit tests.
+    Copy all the tests data to the test dir for the unit tests.
     This avoids polluting the normal working dir with test result artifacts.
     '''
-    test_dir = str(tmp_path) + "/run"
-    shutil.copytree(TRACIE_DIR, test_dir)
+    test_dir = path.join(str(tmp_path) , "run")
+    # Copy the tests
+    shutil.copytree(path.join(path.dirname(path.realpath(__file__)), "tests"),
+                    path.join(test_dir, "tests"))
     # Change the working dir to the test_dir
     os.chdir(test_dir)
-    # Set the traces-db
-    shutil.move("./tests/test-data", "./traces-db")
-    # Disable trace storing
-    os.environ["TRACIE_STORE_IMAGES"] = "0"
+    # Set the replayer-db
+    shutil.move("./tests/test-data", "./replayer-db/")
 
 
 def cleanup(tmp_path):
@@ -85,7 +120,7 @@ def run_test(tmp_path):
     cleanup(tmp_path)
 
 
-def check_results_yaml_content(filename, expectations):
+def check_test_output(filename, expectations):
     '''
     Checks the content of the filename with the list of expectations
     passed as parameter.
@@ -107,110 +142,110 @@ def check_results_yaml_content(filename, expectations):
     return True
 
 
-def test_tracie_succeeds_if_all_images_match():
-    assert run_tracie()
+def test_compare_succeeds_if_all_images_match():
+    assert run_compare([]) is Result.MATCH
     expectations = [
         "actual: 5efda83854befe0155ff8517a58d5b51",
         "expected: 5efda83854befe0155ff8517a58d5b51",
     ]
-    assert check_results_yaml_content(RESULTS_YAML, expectations)
+    assert check_test_output(TESTS_OUTPUT, expectations)
 
 
-def test_tracie_fails_on_image_mismatch():
+def test_compare_fails_on_image_mismatch():
     filename = "./tests/traces.yml"
     content = read_from(filename)
     content = content.replace("5efda83854befe0155ff8517a58d5b51",
                               "8e0a801367e1714463475a824dab363b")
     write_to(content, filename)
-    assert not run_tracie()
+    assert run_compare([]) is Result.DIFFER
     expectations = [
         "actual: 5efda83854befe0155ff8517a58d5b51",
         "expected: 8e0a801367e1714463475a824dab363b",
-        "trace2/test/vk-test-device/olive.testtrace-0.png"
+        "trace/vk-test-device/trace2/olive.testtrace-0.png"
     ]
-    assert check_results_yaml_content(RESULTS_YAML, expectations)
+    assert check_test_output(TESTS_OUTPUT, expectations)
 
 
-def test_tracie_traces_with_and_without_checksum():
+def test_compare_traces_with_and_without_checksum():
     filename = "./tests/traces.yml"
     content = read_from(filename)
-    content += '''  - path: trace1/red.testtrace
-    expectations:
-    - device: bla
-      checksum: 000000000000000'''
+    extra = textwrap.dedent('''\
+    - path: trace1/red.testtrace
+      expectations:
+        - device: blah
+          checksum: 000000000000000
+    ''')
+    content += textwrap.indent(extra, '  ')
     write_to(content, filename)
 
     # red.testtrace should be skipped, since it doesn't
     # have any checksums for our device
-    filename = "./traces-db/trace1/red.testtrace"
+    filename = "./replayer-db/trace1/red.testtrace"
     content = "ff0000ff"
     write_to(content, filename)
-    assert run_tracie()
+    assert run_compare([]) is Result.MATCH
 
 
-def test_tracie_only_traces_without_checksum():
+def test_compare_only_traces_without_checksum():
     filename = "./tests/traces.yml"
-    content = '''traces:
-  - path: trace1/red.testtrace
-    expectations:
-    - device: bla
-      checksum: 000000000000000'''
+    content = textwrap.dedent('''\
+    traces:
+      - path: trace1/red.testtrace
+        expectations:
+          - device: blah
+            checksum: 000000000000000
+    ''')
     write_to(content, filename)
 
     # red.testtrace should be skipped, since it doesn't
     # have any checksums for our device
-    filename = "./traces-db/trace1/red.testtrace"
+    filename = "./replayer-db/trace1/red.testtrace"
     content = "ff0000ff"
     write_to(content, filename)
-    assert run_tracie()
+    assert run_compare([]) is Result.MATCH
 
 
-def test_tracie_with_no_traces():
+def test_compare_with_no_traces():
     filename = "./tests/traces.yml"
     content = 'traces:'
     write_to(content, filename)
-    assert run_tracie()
-    expectations = [
-        "{}",
-    ]
-    assert check_results_yaml_content(RESULTS_YAML, expectations)
+    assert run_compare([]) is Result.MATCH
+    # Check the file is empty
+    assert len(read_from(TESTS_OUTPUT)) == 0
 
 
-def test_tracie_fails_on_dump_image_error():
+def test_compare_fails_on_dump_image_error():
     # "invalid" should fail to parse as rgba and
     # cause an error
-    filename = "./traces-db/trace1/magenta.testtrace"
+    filename = "./replayer-db/trace1/magenta.testtrace"
     write_to("invalid\n", filename)
-    run_tracie()
+    run_compare([])
     expectations = [
         "actual: error",
         "expected: 8e0a801367e1714463475a824dab363b",
         "trace1/magenta.testtrace",
     ]
-    assert check_results_yaml_content(RESULTS_YAML, expectations)
+    assert check_test_output(TESTS_OUTPUT, expectations)
 
 
-def test_tracie_stores_only_logs_on_checksum_match():
-    assert run_tracie()
-    assert path.exists(TRACE_LOG_TEST1)
-    assert path.exists(TRACE_LOG_TEST2)
+def test_compare_stores_only_logs_on_checksum_match():
+    assert run_compare([]) is Result.MATCH
     assert not path.exists(TRACE_PNG_TEST1)
     assert not path.exists(TRACE_PNG_TEST2)
 
 
-def test_tracie_stores_images_on_checksum_mismatch():
+def test_compare_stores_images_on_checksum_mismatch():
     filename = "./tests/traces.yml"
     content = read_from(filename)
     content = content.replace("5efda83854befe0155ff8517a58d5b51",
                               "8e0a801367e1714463475a824dab363b")
     write_to(content, filename)
-    assert not run_tracie()
+    assert run_compare([]) is Result.DIFFER
     assert not path.exists(TRACE_PNG_TEST1)
     assert path.exists(TRACE_PNG_TEST2)
 
 
-def test_tracie_stores_images_on_request():
-    os.environ["TRACIE_STORE_IMAGES"] = "1"
-    assert run_tracie()
+def test_compare_stores_images_on_request():
+    assert run_compare(["--keep-image"]) is Result.MATCH
     assert path.exists(TRACE_PNG_TEST1)
     assert path.exists(TRACE_PNG_TEST2)