From a2ca2aa7d90c091aea4d72df81237fa75843e212 Mon Sep 17 00:00:00 2001
From: Martin Griffin <martinrgriffin@gmail.com>
Date: Thu, 27 Apr 2023 09:25:04 +0100
Subject: [PATCH] Report errors in estimateCost

---
 test/test_runner.c | 88 ++++++++++++++++++++++++++++------------------
 1 file changed, 53 insertions(+), 35 deletions(-)

diff --git a/test/test_runner.c b/test/test_runner.c
index 64cb20e26..bfd7b47a5 100644
--- a/test/test_runner.c
+++ b/test/test_runner.c
@@ -42,7 +42,14 @@ static bool32 PrefixMatch(const char *pattern, const char *string)
     }
 }
 
-enum { STATE_INIT, STATE_NEXT_TEST, STATE_REPORT_RESULT, STATE_EXIT };
+enum
+{
+    STATE_INIT,
+    STATE_NEXT_TEST,
+    STATE_RUN_TEST,
+    STATE_REPORT_RESULT,
+    STATE_EXIT,
+};
 
 void CB2_TestRunner(void)
 {
@@ -81,6 +88,26 @@ void CB2_TestRunner(void)
             return;
         }
 
+        MgbaPrintf_(":N%s", gTestRunnerState.test->name);
+        gTestRunnerState.result = TEST_RESULT_PASS;
+        gTestRunnerState.expectedResult = TEST_RESULT_PASS;
+        gTestRunnerState.expectLeaks = FALSE;
+        if (gTestRunnerHeadless)
+            gTestRunnerState.timeoutSeconds = TIMEOUT_SECONDS;
+        else
+            gTestRunnerState.timeoutSeconds = UINT_MAX;
+        InitHeap(gHeap, HEAP_SIZE);
+        EnableInterrupts(INTR_FLAG_TIMER2);
+        REG_TM2CNT_L = UINT16_MAX - (274 * 60); // Approx. 1 second.
+        REG_TM2CNT_H = TIMER_ENABLE | TIMER_INTR_ENABLE | TIMER_1024CLK;
+
+        // NOTE: Assumes that the compiler interns __FILE__.
+        if (gTestRunnerState.skipFilename == gTestRunnerState.test->filename)
+        {
+            gTestRunnerState.result = TEST_RESULT_ASSUMPTION_FAIL;
+            return;
+        }
+
         // Greedily assign tests to processes based on estimated cost.
         // TODO: Make processCosts a min heap.
         if (gTestRunnerState.test->runner != &gAssumptionsRunner)
@@ -98,40 +125,26 @@ void CB2_TestRunner(void)
                 }
             }
 
+            if (minCostProcess == gTestRunnerI)
+                gTestRunnerState.state = STATE_RUN_TEST;
+            else
+                gTestRunnerState.state = STATE_NEXT_TEST;
+
+            // XXX: If estimateCost exits only on some processes then
+            // processCosts will be inconsistent.
             if (gTestRunnerState.test->runner->estimateCost)
                 gTestRunnerState.processCosts[minCostProcess] += gTestRunnerState.test->runner->estimateCost(gTestRunnerState.test->data);
             else
                 gTestRunnerState.processCosts[minCostProcess] += 1;
-
-            if (minCostProcess != gTestRunnerI)
-                return;
         }
 
-        MgbaPrintf_(":N%s", gTestRunnerState.test->name);
+        break;
+
+    case STATE_RUN_TEST:
         gTestRunnerState.state = STATE_REPORT_RESULT;
-        gTestRunnerState.result = TEST_RESULT_PASS;
-        gTestRunnerState.expectedResult = TEST_RESULT_PASS;
-        gTestRunnerState.expectLeaks = FALSE;
-        if (gTestRunnerHeadless)
-            gTestRunnerState.timeoutSeconds = TIMEOUT_SECONDS;
-        else
-            gTestRunnerState.timeoutSeconds = UINT_MAX;
-        InitHeap(gHeap, HEAP_SIZE);
-        EnableInterrupts(INTR_FLAG_TIMER2);
-        REG_TM2CNT_L = UINT16_MAX - (274 * 60); // Approx. 1 second.
-        REG_TM2CNT_H = TIMER_ENABLE | TIMER_INTR_ENABLE | TIMER_1024CLK;
-
-        // NOTE: Assumes that the compiler interns __FILE__.
-        if (gTestRunnerState.skipFilename == gTestRunnerState.test->filename)
-        {
-            gTestRunnerState.result = TEST_RESULT_ASSUMPTION_FAIL;
-        }
-        else
-        {
-            if (gTestRunnerState.test->runner->setUp)
-                gTestRunnerState.test->runner->setUp(gTestRunnerState.test->data);
-            gTestRunnerState.test->runner->run(gTestRunnerState.test->data);
-        }
+        if (gTestRunnerState.test->runner->setUp)
+            gTestRunnerState.test->runner->setUp(gTestRunnerState.test->data);
+        gTestRunnerState.test->runner->run(gTestRunnerState.test->data);
         break;
 
     case STATE_REPORT_RESULT:
@@ -345,6 +358,8 @@ static void Intr_Timer2(void)
         }
         else
         {
+            if (gTestRunnerState.state == STATE_RUN_TEST)
+                gTestRunnerState.state = STATE_REPORT_RESULT;
             gTestRunnerState.result = TEST_RESULT_TIMEOUT;
             ReinitCallbacks();
             IRQ_LR = ((uintptr_t)JumpToAgbMainLoop & ~1) + 4;
@@ -354,16 +369,19 @@ static void Intr_Timer2(void)
 
 void Test_ExitWithResult(enum TestResult result, const char *fmt, ...)
 {
-    bool32 handled = FALSE;
     gTestRunnerState.result = result;
     ReinitCallbacks();
-    if (gTestRunnerState.test->runner->handleExitWithResult)
-        handled = gTestRunnerState.test->runner->handleExitWithResult(gTestRunnerState.test->data, result);
-    if (!handled && gTestRunnerState.result != gTestRunnerState.expectedResult)
+    if (gTestRunnerState.state == STATE_REPORT_RESULT
+     && gTestRunnerState.test->runner->handleExitWithResult)
     {
-        va_list va;
-        va_start(va, fmt);
-        MgbaVPrintf_(fmt, va);
+        if (!gTestRunnerState.test->runner->handleExitWithResult(gTestRunnerState.test->data, result)
+         && gTestRunnerState.result != gTestRunnerState.expectedResult)
+        {
+            va_list va;
+            va_start(va, fmt);
+            MgbaVPrintf_(fmt, va);
+            va_end(va);
+        }
     }
     JumpToAgbMainLoop();
 }