From b31f10d124709c29e2f40238a0756bc83b384896 Mon Sep 17 00:00:00 2001 From: Martin Griffin Date: Fri, 14 Jul 2023 18:10:42 +0100 Subject: [PATCH 1/4] Make Hydra respect -jN (#3132) --- test/test_battle.h | 2 +- tools/mgba-rom-test-hydra/main.c | 27 ++++++++++++++++++++++++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/test/test_battle.h b/test/test_battle.h index 607589eea..039506c1b 100644 --- a/test/test_battle.h +++ b/test/test_battle.h @@ -1,7 +1,7 @@ /* Embedded DSL for automated black-box testing of battle mechanics. * * To run all the tests use: - * make check + * make check -j * To run specific tests, e.g. Spikes ones, use: * make check TESTS='Spikes' * To build a ROM (pokemerald-test.elf) that can be opened in mgba to diff --git a/tools/mgba-rom-test-hydra/main.c b/tools/mgba-rom-test-hydra/main.c index 7151255de..123d8a255 100644 --- a/tools/mgba-rom-test-hydra/main.c +++ b/tools/mgba-rom-test-hydra/main.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -37,6 +38,8 @@ #define MAX_FAILED_TESTS_TO_LIST 100 #define MAX_TEST_LIST_BUFFER_LENGTH 256 +#define ARRAY_COUNT(arr) (sizeof((arr)) / sizeof((arr)[0])) + struct Runner { pid_t pid; @@ -248,7 +251,29 @@ int main(int argc, char *argv[]) exit(2); } - nrunners = sysconf(_SC_NPROCESSORS_ONLN); + nrunners = 1; + const char *makeflags = getenv("MAKEFLAGS"); + if (makeflags) + { + int e; + regex_t preg; + regmatch_t pmatch[4]; + if ((e = regcomp(&preg, "(^| )-j([0-9]*)($| )", REG_EXTENDED)) != 0) + { + char errbuf[256]; + regerror(e, &preg, errbuf, sizeof(errbuf)); + fprintf(stderr, "regcomp failed: '%s'\n", errbuf); + exit(2); + } + if (regexec(&preg, makeflags, ARRAY_COUNT(pmatch), pmatch, 0) != REG_NOMATCH) + { + if (pmatch[2].rm_so == pmatch[2].rm_eo) + nrunners = sysconf(_SC_NPROCESSORS_ONLN); + else + sscanf(makeflags + pmatch[2].rm_so, "%d", &nrunners); + } + regfree(&preg); + } if (nrunners > MAX_PROCESSES) nrunners = MAX_PROCESSES; runners_digits = ceil(log10(nrunners)); From 4637d7e7eeb4a2b3c4e2231434317fe81691cdaa Mon Sep 17 00:00:00 2001 From: Martin Griffin Date: Fri, 14 Jul 2023 09:50:55 +0100 Subject: [PATCH 2/4] Recover from test crashes --- ld_script_test.txt | 7 +++ test/test.h | 3 +- test/test_runner.c | 131 +++++++++++++++++++++++++++++++++------------ 3 files changed, 104 insertions(+), 37 deletions(-) diff --git a/ld_script_test.txt b/ld_script_test.txt index 3fcac51d4..f7a74e086 100644 --- a/ld_script_test.txt +++ b/ld_script_test.txt @@ -45,6 +45,13 @@ SECTIONS { test/*.o(COMMON); *libc.a:sbrkr.o(COMMON); end = .; + + /* .persistent starts at 0x3007F00 */ + /* WARNING: This is the end of the IRQ stack, if there's too + * much data it WILL be overwritten. */ + . = 0x7F00; + test/*.o(.persistent); + . = 0x8000; } diff --git a/test/test.h b/test/test.h index 3949bdb49..24abca60d 100644 --- a/test/test.h +++ b/test/test.h @@ -13,6 +13,7 @@ enum TestResult TEST_RESULT_INVALID, TEST_RESULT_ERROR, TEST_RESULT_TIMEOUT, + TEST_RESULT_CRASH, TEST_RESULT_TODO, }; @@ -38,8 +39,6 @@ struct TestRunnerState { u8 state; u8 exitCode; - s32 tests; - s32 passes; const char *skipFilename; const struct Test *test; u32 processCosts[MAX_PROCESSES]; diff --git a/test/test_runner.c b/test/test_runner.c index 450dc77a6..5543fab04 100644 --- a/test/test_runner.c +++ b/test/test_runner.c @@ -15,6 +15,16 @@ void CB2_TestRunner(void); EWRAM_DATA struct TestRunnerState gTestRunnerState; EWRAM_DATA struct FunctionTestRunnerState *gFunctionTestRunnerState; +enum { + CURRENT_TEST_STATE_ESTIMATE, + CURRENT_TEST_STATE_RUN, +}; + +__attribute__((section(".persistent"))) static struct { + u32 address:28; + u32 state:1; +} sCurrentTest = {0}; + void TestRunner_Battle(const struct Test *); static bool32 MgbaOpen_(void); @@ -51,6 +61,47 @@ enum STATE_EXIT, }; +static u32 MinCostProcess(void) +{ + u32 i; + u32 minCost, minCostProcess; + + minCost = gTestRunnerState.processCosts[0]; + minCostProcess = 0; + for (i = 1; i < gTestRunnerN; i++) + { + if (gTestRunnerState.processCosts[i] < minCost) + { + minCost = gTestRunnerState.processCosts[i]; + minCostProcess = i; + } + } + + return minCostProcess; +} + +// Greedily assign tests to processes based on estimated cost. +// TODO: Make processCosts a min heap. +static u32 AssignCostToRunner(void) +{ + u32 minCostProcess; + + if (gTestRunnerState.test->runner == &gAssumptionsRunner) + return TRUE; + + minCostProcess = MinCostProcess(); + + // XXX: If estimateCost returns only on some processes, or + // returns inconsistent results then processCosts will be + // inconsistent and some tests may not run. + if (gTestRunnerState.test->runner->estimateCost) + gTestRunnerState.processCosts[minCostProcess] += gTestRunnerState.test->runner->estimateCost(gTestRunnerState.test->data); + else + gTestRunnerState.processCosts[minCostProcess] += 1; + + return minCostProcess; +} + void CB2_TestRunner(void) { switch (gTestRunnerState.state) @@ -65,12 +116,43 @@ void CB2_TestRunner(void) gIntrTable[7] = Intr_Timer2; - gTestRunnerState.state = STATE_NEXT_TEST; + // The current test restarted the ROM (e.g. by jumping to NULL). + if (sCurrentTest.address != 0) + { + gTestRunnerState.test = __start_tests; + while ((uintptr_t)gTestRunnerState.test != sCurrentTest.address) + { + AssignCostToRunner(); + gTestRunnerState.test++; + } + if (sCurrentTest.state == CURRENT_TEST_STATE_ESTIMATE) + { + u32 runner = MinCostProcess(); + gTestRunnerState.processCosts[runner] += 1; + if (runner == gTestRunnerI) + { + gTestRunnerState.state = STATE_REPORT_RESULT; + gTestRunnerState.result = TEST_RESULT_CRASH; + } + else + { + gTestRunnerState.state = STATE_NEXT_TEST; + } + } + else + { + gTestRunnerState.state = STATE_REPORT_RESULT; + gTestRunnerState.result = TEST_RESULT_CRASH; + } + } + else + { + gTestRunnerState.state = STATE_NEXT_TEST; + gTestRunnerState.test = __start_tests - 1; + } gTestRunnerState.exitCode = 0; - gTestRunnerState.tests = 0; - gTestRunnerState.passes = 0; gTestRunnerState.skipFilename = NULL; - gTestRunnerState.test = __start_tests - 1; + break; case STATE_NEXT_TEST: @@ -108,40 +190,19 @@ void CB2_TestRunner(void) return; } - // Greedily assign tests to processes based on estimated cost. - // TODO: Make processCosts a min heap. - if (gTestRunnerState.test->runner != &gAssumptionsRunner) - { - u32 i; - u32 minCost, minCostProcess; - minCost = gTestRunnerState.processCosts[0]; - minCostProcess = 0; - for (i = 1; i < gTestRunnerN; i++) - { - if (gTestRunnerState.processCosts[i] < minCost) - { - minCost = gTestRunnerState.processCosts[i]; - minCostProcess = i; - } - } + sCurrentTest.address = (uintptr_t)gTestRunnerState.test; + sCurrentTest.state = CURRENT_TEST_STATE_ESTIMATE; - 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 (AssignCostToRunner() == gTestRunnerI) + gTestRunnerState.state = STATE_RUN_TEST; + else + gTestRunnerState.state = STATE_NEXT_TEST; break; case STATE_RUN_TEST: gTestRunnerState.state = STATE_REPORT_RESULT; + sCurrentTest.state = CURRENT_TEST_STATE_RUN; if (gTestRunnerState.test->runner->setUp) gTestRunnerState.test->runner->setUp(gTestRunnerState.test->data); gTestRunnerState.test->runner->run(gTestRunnerState.test->data); @@ -186,11 +247,8 @@ void CB2_TestRunner(void) const char *color; const char *result; - gTestRunnerState.tests++; - if (gTestRunnerState.result == gTestRunnerState.expectedResult) { - gTestRunnerState.passes++; color = "\e[32m"; MgbaPrintf_(":N%s", gTestRunnerState.test->name); } @@ -243,6 +301,9 @@ void CB2_TestRunner(void) case TEST_RESULT_TIMEOUT: result = "TIMEOUT"; break; + case TEST_RESULT_CRASH: + result = "CRASH"; + break; default: result = "UNKNOWN"; break; From 25986be0893f8b60b39e277b740f94e1cf4a7110 Mon Sep 17 00:00:00 2001 From: Martin Griffin Date: Fri, 14 Jul 2023 13:27:50 +0100 Subject: [PATCH 3/4] Prevent corrupted heaps causing infinite loops --- test/test_runner.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/test_runner.c b/test/test_runner.c index 5543fab04..d91ad02d1 100644 --- a/test/test_runner.c +++ b/test/test_runner.c @@ -223,6 +223,15 @@ void CB2_TestRunner(void) const struct MemBlock *block = head; do { + if (block->magic != MALLOC_SYSTEM_ID + || !(EWRAM_START <= (uintptr_t)block->next && (uintptr_t)block->next < EWRAM_END) + || (block->next <= block && block->next != head)) + { + MgbaPrintf_("gHeap corrupted block at 0x%p", block); + gTestRunnerState.result = TEST_RESULT_ERROR; + break; + } + if (block->allocated) { const char *location = MemBlockLocation(block); @@ -495,6 +504,7 @@ static s32 MgbaVPrintf_(const char *fmt, va_list va) { s32 i = 0; s32 c, d; + u32 p; const char *s; while (*fmt) { @@ -528,6 +538,20 @@ static s32 MgbaVPrintf_(const char *fmt, va_list va) i = MgbaPutchar_(i, buffer[--n]); } break; + case 'p': + p = va_arg(va, unsigned); + { + s32 n; + for (n = 0; n < 7; n++) + { + unsigned nybble = (p >> (24 - (4*n))) & 0xF; + if (nybble <= 9) + i = MgbaPutchar_(i, '0' + nybble); + else + i = MgbaPutchar_(i, 'a' + nybble - 10); + } + } + break; case 'q': d = va_arg(va, int); { From b48175edd2c04009c57e6adac1772427867154d7 Mon Sep 17 00:00:00 2001 From: Eduardo Quezada D'Ottone Date: Sat, 15 Jul 2023 11:25:23 -0400 Subject: [PATCH 4/4] Updated branches that use CI --- .github/workflows/build.yml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index a2b93f23b..02d7e22b6 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -4,9 +4,7 @@ on: push: branches: - master - - battle_engine - - pokemon_expansion - - item_expansion + - upcoming pull_request: jobs: