Separate build directory for tests and TESTING define (#3002)

* Separate build directory for tests and TESTING define

* fixup! Separate build directory for tests and TESTING define

* fixup! Separate build directory for tests and TESTING define

* fixup! Separate build directory for tests and TESTING define

.github/workflows/build.yml

@@ -50,6 +50,8 @@ jobs:
         run: make -j${nproc} -O all
 
       - name: Test
+        env:
+          TEST: 1
         run: |
           make -j${nproc} -O pokeemerald-test.elf
           make -j${nproc} check

Makefile

@@ -41,11 +41,16 @@ GAME_CODE   := BPEE
 MAKER_CODE  := 01
 REVISION    := 0
 MODERN      ?= 0
+TEST        ?= 0
 
 ifeq (modern,$(MAKECMDGOALS))
   MODERN := 1
 endif
 
+ifeq (check,$(MAKECMDGOALS))
+  TEST := 1
+endif
+
 # use arm-none-eabi-cpp for macOS
 # as macOS's default compiler is clang
 # and clang's preprocessor will warn on \u
@@ -79,6 +84,7 @@ ELF = $(ROM:.gba=.elf)
 MAP = $(ROM:.gba=.map)
 SYM = $(ROM:.gba=.sym)
 
+TEST_OBJ_DIR_NAME := build/test
 TESTELF = $(ROM:.gba=-test.elf)
 HEADLESSELF = $(ROM:.gba=-test-headless.elf)
 
@@ -119,7 +125,15 @@ LIBPATH := -L "$(dir $(shell $(PATH_MODERNCC) -mthumb -print-file-name=libgcc.a)
 LIB := $(LIBPATH) -lc -lnosys -lgcc -L../../libagbsyscall -lagbsyscall
 endif
 
-CPPFLAGS := -iquote include -iquote $(GFLIB_SUBDIR) -Wno-trigraphs -DMODERN=$(MODERN)
+ifeq ($(TESTELF),$(MAKECMDGOALS))
+  TEST := 1
+endif
+
+ifeq ($(TEST),1)
+OBJ_DIR := $(TEST_OBJ_DIR_NAME)
+endif
+
+CPPFLAGS := -iquote include -iquote $(GFLIB_SUBDIR) -Wno-trigraphs -DMODERN=$(MODERN) -DTESTING=$(TEST)
 ifneq ($(MODERN),1)
 CPPFLAGS += -I tools/agbcc/include -I tools/agbcc -nostdinc -undef
 endif
@@ -168,7 +182,7 @@ infoshell = $(foreach line, $(shell $1 | sed "s/ /__SPACE__/g"), $(info $(subst
 # Disable dependency scanning for clean/tidy/tools
 # Use a separate minimal makefile for speed
 # Since we don't need to reload most of this makefile
-ifeq (,$(filter-out all rom compare modern check libagbsyscall syms,$(MAKECMDGOALS)))
+ifeq (,$(filter-out all rom compare modern check libagbsyscall syms $(TESTELF),$(MAKECMDGOALS)))
 $(call infoshell, $(MAKE) -f make_tools.mk)
 else
 NODEP ?= 1
@@ -178,9 +192,9 @@ endif
 ifeq (,$(MAKECMDGOALS))
   SCAN_DEPS ?= 1
 else
-  # clean, tidy, tools, check-tools, mostlyclean, clean-tools, clean-check-tools, $(TOOLDIRS), $(CHECKTOOLDIRS), tidymodern, tidynonmodern don't even build the ROM
+  # clean, tidy, tools, check-tools, mostlyclean, clean-tools, clean-check-tools, $(TOOLDIRS), $(CHECKTOOLDIRS), tidymodern, tidynonmodern, tidycheck don't even build the ROM
   # libagbsyscall does its own thing
-  ifeq (,$(filter-out clean tidy tools mostlyclean clean-tools $(TOOLDIRS) clean-check-tools $(CHECKTOOLDIRS) tidymodern tidynonmodern libagbsyscall,$(MAKECMDGOALS)))
+  ifeq (,$(filter-out clean tidy tools mostlyclean clean-tools $(TOOLDIRS) clean-check-tools $(CHECKTOOLDIRS) tidymodern tidynonmodern tidycheck libagbsyscall,$(MAKECMDGOALS)))
     SCAN_DEPS ?= 0
   else
     SCAN_DEPS ?= 1
@@ -257,7 +271,7 @@ clean-tools:
 clean-check-tools:
 	@$(foreach tooldir,$(CHECKTOOLDIRS),$(MAKE) clean -C $(tooldir);)
 
-mostlyclean: tidynonmodern tidymodern
+mostlyclean: tidynonmodern tidymodern tidycheck
 	rm -f $(SAMPLE_SUBDIR)/*.bin
 	rm -f $(CRY_SUBDIR)/*.bin
 	rm -f $(MID_SUBDIR)/*.s
@@ -268,7 +282,7 @@ mostlyclean: tidynonmodern tidymodern
 	rm -f $(AUTO_GEN_TARGETS)
 	@$(MAKE) clean -C libagbsyscall
 
-tidy: tidynonmodern tidymodern
+tidy: tidynonmodern tidymodern tidycheck
 
 tidynonmodern:
 	rm -f $(ROM_NAME) $(ELF_NAME) $(MAP_NAME)
@@ -278,6 +292,10 @@ tidymodern:
 	rm -f $(MODERN_ROM_NAME) $(MODERN_ELF_NAME) $(MODERN_MAP_NAME)
 	rm -rf $(MODERN_OBJ_DIR_NAME)
 
+tidycheck:
+	rm -f $(TESTELF) $(HEADLESSELF)
+	rm -rf $(TEST_OBJ_DIR_NAME)
+
 ifneq ($(MODERN),0)
 $(C_BUILDDIR)/berry_crush.o: override CFLAGS += -Wno-address-of-packed-member
 endif

gflib/malloc.h

@@ -44,9 +44,18 @@ struct MemBlock
 
 extern u8 gHeap[];
 
+#if TESTING || !defined(NDEBUG)
+
 #define Alloc(size) Alloc_(size, __FILE__ ":" STR(__LINE__))
 #define AllocZeroed(size) AllocZeroed_(size, __FILE__ ":" STR(__LINE__))
 
+#else
+
+#define Alloc(size) Alloc_(size, NULL)
+#define AllocZeroed(size) AllocZeroed_(size, NULL)
+
+#endif
+
 void *Alloc_(u32 size, const char *location);
 void *AllocZeroed_(u32 size, const char *location);
 void Free(void *pointer);

include/test_runner.h

@@ -3,6 +3,9 @@
 
 extern const bool8 gTestRunnerEnabled;
 extern const bool8 gTestRunnerHeadless;
+
+#if TESTING
+
 extern const bool8 gTestRunnerSkipIsFail;
 
 void TestRunner_Battle_RecordAbilityPopUp(u32 battlerId, u32 ability);
@@ -12,6 +15,19 @@ void TestRunner_Battle_RecordMessage(const u8 *message);
 void TestRunner_Battle_RecordStatus1(u32 battlerId, u32 status1);
 void TestRunner_Battle_AfterLastTurn(void);
 
-void BattleTest_CheckBattleRecordActionType(u32 battlerId, u32 recordIndex, u32 actionType);
+void TestRunner_Battle_CheckBattleRecordActionType(u32 battlerId, u32 recordIndex, u32 actionType);
+
+#else
+
+#define TestRunner_Battle_RecordAbilityPopUp(...) (void)0
+#define TestRunner_Battle_RecordAnimation(...) (void)0
+#define TestRunner_Battle_RecordHP(...) (void)0
+#define TestRunner_Battle_RecordMessage(...) (void)0
+#define TestRunner_Battle_RecordStatus1(...) (void)0
+#define TestRunner_Battle_AfterLastTurn(...) (void)0
+
+#define TestRunner_Battle_CheckBattleRecordActionType(...) (void)0
+
+#endif
 
 #endif

src/recorded_battle.c

@@ -176,7 +176,7 @@ void RecordedBattle_ClearBattlerAction(u8 battlerId, u8 bytesToClear)
 u8 RecordedBattle_GetBattlerAction(u32 actionType, u8 battlerId)
 {
     if (gTestRunnerEnabled)
-        BattleTest_CheckBattleRecordActionType(battlerId, sBattlerRecordSizes[battlerId], actionType);
+        TestRunner_Battle_CheckBattleRecordActionType(battlerId, sBattlerRecordSizes[battlerId], actionType);
 
     // Trying to read past array or invalid action byte, battle is over.
     if (sBattlerRecordSizes[battlerId] >= BATTLER_RECORD_SIZE || sBattleRecords[battlerId][sBattlerRecordSizes[battlerId]] == 0xFF)

src/test_runner_stub.c

@@ -9,38 +9,3 @@ const bool8 gTestRunnerEnabled = FALSE;
 // animations and messages play, which helps when debugging a test.
 const bool8 gTestRunnerHeadless = FALSE;
 const bool8 gTestRunnerSkipIsFail = FALSE;
-
-__attribute__((weak))
-void TestRunner_Battle_RecordAbilityPopUp(u32 battlerId, u32 ability)
-{
-}
-
-__attribute__((weak))
-void TestRunner_Battle_RecordAnimation(u32 animType, u32 animId)
-{
-}
-
-__attribute__((weak))
-void TestRunner_Battle_RecordHP(u32 battlerId, u32 oldHP, u32 newHP)
-{
-}
-
-__attribute__((weak))
-void TestRunner_Battle_RecordMessage(const u8 *string)
-{
-}
-
-__attribute__((weak))
-void TestRunner_Battle_RecordStatus1(u32 battlerId, u32 status1)
-{
-}
-
-__attribute__((weak))
-void TestRunner_Battle_AfterLastTurn(void)
-{
-}
-
-__attribute__((weak))
-void BattleTest_CheckBattleRecordActionType(u32 battlerId, u32 recordIndex, u32 actionType)
-{
-}

test/test_runner_battle.c

@@ -1241,7 +1241,7 @@ static void PushBattlerAction(u32 sourceLine, s32 battlerId, u32 actionType, u32
     DATA.recordedBattle.battleRecord[battlerId][recordIndex] = byte;
 }
 
-void BattleTest_CheckBattleRecordActionType(u32 battlerId, u32 recordIndex, u32 actionType)
+void TestRunner_Battle_CheckBattleRecordActionType(u32 battlerId, u32 recordIndex, u32 actionType)
 {
     // An illegal move choice will cause the battle to request a new
     // move slot and target. This detects the move slot.