tests: Cleanup decompiler test fixtures and try to improve the test o…

…utput / add a readable report (#1724)

* tests: add a Jak2 `FormRegressionTest` fixture

* tests/ci: minimize test log output and produce a readable report

* improve naming of report due to underlying GitHub issue

* hopefully fix windows handling of this argument

* test...

* final test

* test linux coverage

.github/workflows/linux-build-clang.yaml

@@ -52,8 +52,18 @@ jobs:
         run: cmake --build build -j$((`nproc`+1))
 
       - name: Run Tests
+        env:
+          GTEST_OUTPUT: "xml:opengoal-test-report.xml"
         run: ./test.sh
 
+      - name: Generate Test Report
+        uses: dorny/test-reporter@v1
+        if: success() || failure() # run this step even if previous step failed
+        with:
+          name: Linux Clang - Test Report
+          path: ${{ github.workspace }}/**/opengoal-test-report.xml
+          reporter: jest-junit
+
       - name: Upload artifact
         uses: actions/upload-artifact@v3
         with:

.github/workflows/linux-build-gcc.yaml

@@ -54,8 +54,18 @@ jobs:
 
       - name: Run Tests - With Coverage
         working-directory: ./build
+        env:
+          GTEST_OUTPUT: "xml:opengoal-test-report.xml"
         run: ninja goalc-test_coverage -w dupbuild=warn
 
+      - name: Generate Test Report
+        uses: dorny/test-reporter@v1
+        if: success() || failure() # run this step even if previous step failed
+        with:
+          name: Linux GCC - Test Report
+          path: ${{ github.workspace }}/**/opengoal-test-report.xml
+          reporter: jest-junit
+
       - name: Submit Coverage Report to Codacy
         uses: codacy/codacy-coverage-reporter-action@v1
         continue-on-error: true

.github/workflows/windows-build-clang.yaml

@@ -48,7 +48,17 @@ jobs:
 
       - name: Run Tests
         timeout-minutes: 10
-        run: ./build/bin/goalc-test.exe --gtest_color=yes --gtest_filter="-*MANUAL_TEST*"
+        env:
+          GTEST_OUTPUT: "xml:opengoal-test-report.xml"
+        run: ./build/bin/goalc-test.exe --gtest_color=yes --gtest_brief=1 --gtest_filter="-*MANUAL_TEST*"
+
+      - name: Generate Test Report
+        uses: dorny/test-reporter@v1
+        if: success() || failure() # run this step even if previous step failed
+        with:
+          name: Windows Clang - Test Report
+          path: ./opengoal-test-report.xml
+          reporter: jest-junit
 
       - name: Upload artifact
         uses: actions/upload-artifact@v3

.github/workflows/windows-build-msvc.yaml

@@ -49,4 +49,15 @@ jobs:
 
       - name: Run Tests
         timeout-minutes: 10
-        run: ./build/bin/goalc-test.exe --gtest_color=yes --gtest_filter="-*MANUAL_TEST*"
+        env:
+          GTEST_OUTPUT: "xml:opengoal-test-report.xml"
+        run: |
+          ./build/bin/goalc-test.exe --gtest_color=yes --gtest_brief=1 --gtest_filter="-*MANUAL_TEST*"
+
+      - name: Generate Test Report
+        uses: dorny/test-reporter@v1
+        if: success() || failure() # run this step even if previous step failed
+        with:
+          name: Windows MSVC - Test Report
+          path: ./opengoal-test-report.xml
+          reporter: jest-junit

test.sh

@@ -3,4 +3,4 @@
 # Directory of this script
 DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
 
-$DIR/build/goalc-test --gtest_color=yes "$@" --gtest_filter="-*MANUAL_TEST*"
+$DIR/build/goalc-test --gtest_brief=1 --gtest_color=yes "$@" --gtest_filter="-*MANUAL_TEST*"

test/CMakeLists.txt

@@ -29,7 +29,7 @@ add_executable(goalc-test
         ${CMAKE_CURRENT_LIST_DIR}/decompiler/test_FormExpressionBuildLong.cpp
         ${CMAKE_CURRENT_LIST_DIR}/decompiler/test_InstructionDecode.cpp
         ${CMAKE_CURRENT_LIST_DIR}/decompiler/test_InstructionParser.cpp
-        ${CMAKE_CURRENT_LIST_DIR}/decompiler/test_gkernel_decomp.cpp
+        ${CMAKE_CURRENT_LIST_DIR}/decompiler/test_gkernel_jak1_decomp.cpp
         ${CMAKE_CURRENT_LIST_DIR}/decompiler/test_math_decomp.cpp
         ${CMAKE_CURRENT_LIST_DIR}/decompiler/test_DataParser.cpp
         ${CMAKE_CURRENT_LIST_DIR}/decompiler/test_DisasmVifDecompile.cpp
@@ -50,7 +50,7 @@ if(UNIX AND CMAKE_COMPILER_IS_GNUCXX AND CODE_COVERAGE)
   include(CodeCoverage)
   append_coverage_compiler_flags()
   setup_target_for_coverage_lcov(NAME goalc-test_coverage
-                                 EXECUTABLE goalc-test --gtest_color=yes
+                                 EXECUTABLE goalc-test --gtest_color=yes --gtest_brief=1
                                  DEPENDENCIES goalc-test
                                  EXCLUDE "third-party/*" "/usr/include/*")
 endif()

test/decompiler/FormRegressionTest.cpp

@@ -19,12 +19,18 @@
 
 using namespace decompiler;
 
-void FormRegressionTest::SetUpTestCase() {
+void FormRegressionTestJak1::SetUpTestCase() {
   parser = std::make_unique<InstructionParser>();
-  dts = std::make_unique<DecompilerTypeSystem>(GameVersion::Jak1);
+  dts = std::make_unique<DecompilerTypeSystem>(game_version);
   dts->parse_type_defs({"decompiler", "config", "all-types.gc"});
 }
 
+void FormRegressionTestJak2::SetUpTestCase() {
+  parser = std::make_unique<InstructionParser>();
+  dts = std::make_unique<DecompilerTypeSystem>(game_version);
+  dts->parse_type_defs({"decompiler", "config", "jak2", "all-types.gc"});
+}
+
 void FormRegressionTest::TearDownTestCase() {
   parser.reset();
   dts.reset();
@@ -280,7 +286,7 @@ void FormRegressionTest::test(const std::string& code,
   EXPECT_TRUE(expected_form == actual_form);
 }
 
-void FormRegressionTest::test_final_function_jak1(
+void FormRegressionTest::test_final_function(
     const std::string& code,
     const std::string& type,
     const std::string& expected,
@@ -311,12 +317,12 @@ void FormRegressionTest::test_final_function_jak1(
   EXPECT_TRUE(expected_form == actual_form);
 }
 
-void FormRegressionTest::test_with_stack_structures_jak1(const std::string& code,
-                                                         const std::string& type,
-                                                         const std::string& expected,
-                                                         const std::string& stack_map_json,
-                                                         const std::string& cast_json,
-                                                         const std::string& var_map_json) {
+void FormRegressionTest::test_with_stack_structures(const std::string& code,
+                                                    const std::string& type,
+                                                    const std::string& expected,
+                                                    const std::string& stack_map_json,
+                                                    const std::string& cast_json,
+                                                    const std::string& var_map_json) {
   TestSettings settings;
   settings.do_expressions = true;
   settings.stack_structure_json = stack_map_json;

test/decompiler/FormRegressionTest.h

@@ -23,12 +23,11 @@ struct TestSettings {
   GameVersion version = GameVersion::Jak1;
 };
 
-class FormRegressionTest : public ::testing::Test {
+class FormRegressionTest : public ::testing::TestWithParam<GameVersion> {
  protected:
   static std::unique_ptr<decompiler::InstructionParser> parser;
   static std::unique_ptr<decompiler::DecompilerTypeSystem> dts;
 
-  static void SetUpTestCase();
   static void TearDownTestCase();
 
   struct TestData {
@@ -47,23 +46,22 @@ class FormRegressionTest : public ::testing::Test {
             const std::string& expected,
             const TestSettings& settings);
 
-  void test_final_function_jak1(
-      const std::string& code,
-      const std::string& type,
-      const std::string& expected,
-      bool allow_pairs = false,
-      const std::vector<std::pair<std::string, std::string>>& strings = {},
-      const std::string& cast_json = "",
-      const std::string& var_map_json = "");
-
-  void test_no_expr_jak1(const std::string& code,
-                         const std::string& type,
-                         const std::string& expected,
-                         bool allow_pairs = false,
-                         const std::string& method_name = "",
-                         const std::vector<std::pair<std::string, std::string>>& strings = {},
-                         const std::string& cast_json = "",
-                         const std::string& var_map_json = "") {
+  void test_final_function(const std::string& code,
+                           const std::string& type,
+                           const std::string& expected,
+                           bool allow_pairs = false,
+                           const std::vector<std::pair<std::string, std::string>>& strings = {},
+                           const std::string& cast_json = "",
+                           const std::string& var_map_json = "");
+
+  void test_no_expr(const std::string& code,
+                    const std::string& type,
+                    const std::string& expected,
+                    bool allow_pairs = false,
+                    const std::string& method_name = "",
+                    const std::vector<std::pair<std::string, std::string>>& strings = {},
+                    const std::string& cast_json = "",
+                    const std::string& var_map_json = "") {
     TestSettings settings;
     settings.allow_pairs = allow_pairs;
     settings.method_name = method_name;
@@ -74,14 +72,14 @@ class FormRegressionTest : public ::testing::Test {
     test(code, type, expected, settings);
   }
 
-  void test_with_expr_jak1(const std::string& code,
-                           const std::string& type,
-                           const std::string& expected,
-                           bool allow_pairs = false,
-                           const std::string& method_name = "",
-                           const std::vector<std::pair<std::string, std::string>>& strings = {},
-                           const std::string& cast_json = "",
-                           const std::string& var_map_json = "") {
+  void test_with_expr(const std::string& code,
+                      const std::string& type,
+                      const std::string& expected,
+                      bool allow_pairs = false,
+                      const std::string& method_name = "",
+                      const std::vector<std::pair<std::string, std::string>>& strings = {},
+                      const std::string& cast_json = "",
+                      const std::string& var_map_json = "") {
     TestSettings settings;
     settings.allow_pairs = allow_pairs;
     settings.method_name = method_name;
@@ -92,10 +90,24 @@ class FormRegressionTest : public ::testing::Test {
     test(code, type, expected, settings);
   }
 
-  void test_with_stack_structures_jak1(const std::string& code,
-                                       const std::string& type,
-                                       const std::string& expected,
-                                       const std::string& stack_map_json,
-                                       const std::string& cast_json = "",
-                                       const std::string& var_map_json = "");
-};
+  void test_with_stack_structures(const std::string& code,
+                                  const std::string& type,
+                                  const std::string& expected,
+                                  const std::string& stack_map_json,
+                                  const std::string& cast_json = "",
+                                  const std::string& var_map_json = "");
+};
+
+class FormRegressionTestJak1 : public FormRegressionTest {
+ protected:
+  static const GameVersion game_version = GameVersion::Jak1;
+
+  static void SetUpTestCase();
+};
+
+class FormRegressionTestJak2 : public FormRegressionTest {
+ protected:
+  static const GameVersion game_version = GameVersion::Jak2;
+
+  static void SetUpTestCase();
+};

test/decompiler/test_DisasmVifDecompile.cpp

@@ -5,7 +5,7 @@
 
 using namespace decompiler;
 
-TEST_F(FormRegressionTest, ExprDisasmVif) {
+TEST_F(FormRegressionTestJak1, ExprDisasmVif) {
   std::string func =
       "    sll r0, r0, 0\n"
       "L53:\n"
@@ -681,17 +681,17 @@ TEST_F(FormRegressionTest, ExprDisasmVif) {
       "   )\n"
       "  (- gp-0 (* arg1 4))\n"
       "  )";
-  test_with_expr_jak1(func, type, expected, false, "",
-                      {{"L139", " #x~X:"},
-                       {"L138", "  (~s :irq ~D)~%"},
-                       {"L137", "  (~s :irq ~D :~s #x~X)~%"},
-                       {"L136", "  (~s :irq ~D :wl ~D :cl ~D)~%"},
-                       {"L135", "  (~s :irq ~D :~s "},
-                       {"L134", "#x~X #x~X #x~X #x~X)~%"},
-                       {"L133", "  (~s :irq ~D :instructions #x~D :addr #x~X)~%"},
-                       {"L132", "  (~s :irq ~D :qwc #x~D)~%"},
-                       {"L145", "  #x~X:    #x~8x #x~8x #x~8x #x~8x~%"},
-                       {"L131", "  (~s :irq ~D :num ~D :addr #x~X "},
-                       {"L130", ":msk ~D :flg ~D :usn ~D [skip ~d])~%"},
-                       {"L129", "  (*unknown* vif-tag #x~X)~%"}});
-}
+  test_with_expr(func, type, expected, false, "",
+                 {{"L139", " #x~X:"},
+                  {"L138", "  (~s :irq ~D)~%"},
+                  {"L137", "  (~s :irq ~D :~s #x~X)~%"},
+                  {"L136", "  (~s :irq ~D :wl ~D :cl ~D)~%"},
+                  {"L135", "  (~s :irq ~D :~s "},
+                  {"L134", "#x~X #x~X #x~X #x~X)~%"},
+                  {"L133", "  (~s :irq ~D :instructions #x~D :addr #x~X)~%"},
+                  {"L132", "  (~s :irq ~D :qwc #x~D)~%"},
+                  {"L145", "  #x~X:    #x~8x #x~8x #x~8x #x~8x~%"},
+                  {"L131", "  (~s :irq ~D :num ~D :addr #x~X "},
+                  {"L130", ":msk ~D :flg ~D :usn ~D [skip ~d])~%"},
+                  {"L129", "  (*unknown* vif-tag #x~X)~%"}});
+}