Makes integration test results be in color and have annotations (#66649)

About The Pull Request

    Separated compiling the integration tests and running them as separate steps for organization purposes.
    Added a TEST_ASSERT_NULL(value, reason) and TEST_ASSERT_NOTNULL(value, reason) because those are conceptually simple tests.
    Makes the PASS and FAIL prefixes in the integration test log be green and red for better readability.
    Failure reasons now display the file and line number.
        In order to achieve this, direct calls to Fail() are now wrapped in a macro, TEST_FAIL(), as Fail() itself needs preprocessor stuff passed to it.
        In the midst of updating it, I noticed multiple cases of tests directly calling Fail() and returning when they should have used a better macro, so those were updated. There was at least one case where it appeared that the code assumed that the test ended at Fail(), but made no attempt to do so, such as with the RCD test.
        Feel free to double check all of the changed unit tests in case I made a functional behavior change, but they currently pass.
    To take advantage of the previous change, failures are now marked as annotations. Note that outside of github, this creates an ugly-looking line but the primary environment is as a github action.

Examples with intentionally botched unit test:

image

image

image
Why It's Good For The Game

Makes inspecting failed unit tests significantly easier.
Changelog

N/A

code/modules/unit_tests/README.md

@@ -51,12 +51,18 @@ Unit tests should also be just that--testing *units* of code. For example, inste
 
 You can find more information about all of these from their respective doc comments, but for a brief overview:
 
-`/datum/unit_test` - The base for all tests to be ran. Subtypes must override `Run()`. `New()` and `Destroy()` can be used for setup and teardown. To fail, use `Fail(reason)`.
+`/datum/unit_test` - The base for all tests to be ran. Subtypes must override `Run()`. `New()` and `Destroy()` can be used for setup and teardown. To fail, use `TEST_FAIL(reason)`.
 
 `/datum/unit_test/proc/allocate(type, ...)` - Allocates an instance of the provided type with the given arguments. Is automatically destroyed when the test is over. Commonly seen in the form of `var/mob/living/carbon/human/human = allocate(/mob/living/carbon/human)`.
 
+`TEST_FAIL(reason)` - Marks a failure at this location, but does not stop the test.
+
 `TEST_ASSERT(assertion, reason)` - Stops the unit test and fails if the assertion is not met. For example: `TEST_ASSERT(powered(), "Machine is not powered")`.
 
+`TEST_ASSERT_NOTNULL(a, message)` - Same as `TEST_ASSERT`, but checks if `!isnull(a)`. For example: `TEST_ASSERT_NOTNULL(myatom, "My atom was never set!")`.
+
+`TEST_ASSERT_NULL(a, message)` - Same as `TEST_ASSERT`, but checks if `isnull(a)`. If not, gives a helpful message showing what `a` was. For example: `TEST_ASSERT_NULL(delme, "Delme was never cleaned up!")`.
+
 `TEST_ASSERT_EQUAL(a, b, message)` - Same as `TEST_ASSERT`, but checks if `a == b`. If not, gives a helpful message showing what both `a` and `b` were. For example: `TEST_ASSERT_EQUAL(2 + 2, 4, "The universe is falling apart before our eyes!")`.
 
 `TEST_ASSERT_NOTEQUAL(a, b, message)` - Same as `TEST_ASSERT_EQUAL`, but reversed.

code/modules/unit_tests/_unit_tests.dm

@@ -3,9 +3,18 @@
 
 #if defined(UNIT_TESTS) || defined(SPACEMAN_DMM)
 
+/// For advanced cases, fail unconditionally but don't return (so a test can return multiple results)
+#define TEST_FAIL(reason) (Fail(reason || "No reason", __FILE__, __LINE__))
+
 /// Asserts that a condition is true
 /// If the condition is not true, fails the test
-#define TEST_ASSERT(assertion, reason) if (!(assertion)) { return Fail("Assertion failed: [reason || "No reason"]") }
+#define TEST_ASSERT(assertion, reason) if (!(assertion)) { return Fail("Assertion failed: [reason || "No reason"]", __FILE__, __LINE__) }
+
+/// Asserts that a parameter is not null
+#define TEST_ASSERT_NOTNULL(a, reason) if (isnull(a)) { return Fail("Expected non-null value: [reason || "No reason"]", __FILE__, __LINE__) }
+
+/// Asserts that a parameter is null
+#define TEST_ASSERT_NULL(a, reason) if (!isnull(a)) { return Fail("Expected null value but received [a]: [reason || "No reason"]", __FILE__, __LINE__) }
 
 /// Asserts that the two parameters passed are equal, fails otherwise
 /// Optionally allows an additional message in the case of a failure
@@ -13,7 +22,7 @@
 	var/lhs = ##a; \
 	var/rhs = ##b; \
 	if (lhs != rhs) { \
-		return Fail("Expected [isnull(lhs) ? "null" : lhs] to be equal to [isnull(rhs) ? "null" : rhs].[message ? " [message]" : ""]"); \
+		return Fail("Expected [isnull(lhs) ? "null" : lhs] to be equal to [isnull(rhs) ? "null" : rhs].[message ? " [message]" : ""]", __FILE__, __LINE__); \
 	} \
 } while (FALSE)
 
@@ -23,7 +32,7 @@
 	var/lhs = ##a; \
 	var/rhs = ##b; \
 	if (lhs == rhs) { \
-		return Fail("Expected [isnull(lhs) ? "null" : lhs] to not be equal to [isnull(rhs) ? "null" : rhs].[message ? " [message]" : ""]"); \
+		return Fail("Expected [isnull(lhs) ? "null" : lhs] to not be equal to [isnull(rhs) ? "null" : rhs].[message ? " [message]" : ""]", __FILE__, __LINE__); \
 	} \
 } while (FALSE)
 
@@ -40,6 +49,19 @@
 #define TEST_DEFAULT 1
 #define TEST_DEL_WORLD INFINITY
 
+/// Change color to red on ANSI terminal output, if enabled with -DANSICOLORS.
+#ifdef ANSICOLORS
+#define TEST_OUTPUT_RED(text) "\x1B\x5B1;31m[text]\x1B\x5B0m"
+#else
+#define TEST_OUTPUT_RED(text) (text)
+#endif
+/// Change color to green on ANSI terminal output, if enabled with -DANSICOLORS.
+#ifdef ANSICOLORS
+#define TEST_OUTPUT_GREEN(text) "\x1B\x5B1;32m[text]\x1B\x5B0m"
+#else
+#define TEST_OUTPUT_GREEN(text) (text)
+#endif
+
 /// A trait source when adding traits through unit tests
 #define TRAIT_SOURCE_UNIT_TESTS "unit_tests"
 

code/modules/unit_tests/achievements.dm

@@ -8,4 +8,4 @@
 			continue
 		var/init_icon = initial(award.icon)
 		if(!init_icon || !(init_icon in award_icons))
-			Fail("Award [initial(award.name)] has an unexistent icon: \"[init_icon || "null"]\"")
+			TEST_FAIL("Award [initial(award.name)] has an unexistent icon: \"[init_icon || "null"]\"")

code/modules/unit_tests/anchored_mobs.dm

@@ -4,6 +4,4 @@
 		var/mob/M = i
 		if(initial(M.anchored))
 			L += "[i]"
-	if(!L.len)
-		return //passed!
-	Fail("The following mobs are defined as anchored. This is incompatible with the new move force/resist system and needs to be revised.: [L.Join(" ")]")
+	TEST_ASSERT(!L.len, "The following mobs are defined as anchored. This is incompatible with the new move force/resist system and needs to be revised.: [L.Join(" ")]")

code/modules/unit_tests/bespoke_id.dm

@@ -5,4 +5,4 @@
 	for(var/i in subtypesof(/datum/element))
 		var/datum/element/faketype = i
 		if((initial(faketype.element_flags) & ELEMENT_BESPOKE) && initial(faketype.id_arg_index) == base_index)
-			Fail("A bespoke element was not configured with a proper id_arg_index: [faketype]")
+			TEST_FAIL("A bespoke element was not configured with a proper id_arg_index: [faketype]")

code/modules/unit_tests/card_mismatch.dm

@@ -3,5 +3,4 @@
 /datum/unit_test/card_mismatch/Run()
 	var/message = SStrading_card_game.checkCardpacks(SStrading_card_game.card_packs)
 	message += SStrading_card_game.checkCardDatums()
-	if(message)
-		Fail(message)
+	TEST_ASSERT(!message, message)

code/modules/unit_tests/chain_pull_through_space.dm

@@ -41,22 +41,15 @@
 
 	// Walk normally to the left, make sure we're still a chain
 	alice.Move(locate(run_loc_floor_bottom_left.x + 1, run_loc_floor_bottom_left.y, run_loc_floor_bottom_left.z))
-	if (bob.x != run_loc_floor_bottom_left.x + 2)
-		return Fail("During normal move, Bob was not at the correct x ([bob.x])")
-	if (charlie.x != run_loc_floor_bottom_left.x + 3)
-		return Fail("During normal move, Charlie was not at the correct x ([charlie.x])")
+	TEST_ASSERT_EQUAL(bob.x, run_loc_floor_bottom_left.x + 2, "During normal move, Bob was not at the correct x ([bob.x])")
+	TEST_ASSERT_EQUAL(charlie.x, run_loc_floor_bottom_left.x + 3, "During normal move, Charlie was not at the correct x ([charlie.x])")
 
 	// We're going through the space turf now that should teleport us
 	alice.Move(run_loc_floor_bottom_left)
-	if (alice.z != space_tile.destination_z)
-		return Fail("Alice did not teleport to the destination z-level. Current location: ([alice.x], [alice.y], [alice.z])")
+	TEST_ASSERT_EQUAL(alice.z, space_tile.destination_z, "Alice did not teleport to the destination z-level. Current location: ([alice.x], [alice.y], [alice.z])")
 
-	if (bob.z != space_tile.destination_z)
-		return Fail("Bob did not teleport to the destination z-level. Current location: ([bob.x], [bob.y], [bob.z])")
-	if (!bob.Adjacent(alice))
-		return Fail("Bob is not adjacent to Alice. Bob is at [bob.x], Alice is at [alice.x]")
+	TEST_ASSERT_EQUAL(bob.z, space_tile.destination_z, "Bob did not teleport to the destination z-level. Current location: ([bob.x], [bob.y], [bob.z])")
+	TEST_ASSERT(bob.Adjacent(alice), "Bob is not adjacent to Alice. Bob is at [bob.x], Alice is at [alice.x]")
 
-	if (charlie.z != space_tile.destination_z)
-		return Fail("Charlie did not teleport to the destination z-level. Current location: ([charlie.x], [charlie.y], [charlie.z])")
-	if (!charlie.Adjacent(bob))
-		return Fail("Charlie is not adjacent to Bob. Charlie is at [charlie.x], Bob is at [bob.x]")
+	TEST_ASSERT_EQUAL(charlie.z, space_tile.destination_z, "Charlie did not teleport to the destination z-level. Current location: ([charlie.x], [charlie.y], [charlie.z])")
+	TEST_ASSERT(charlie.Adjacent(bob), "Charlie is not adjacent to Bob. Charlie is at [charlie.x], Bob is at [bob.x]")

code/modules/unit_tests/chat_filter.dm

@@ -92,20 +92,17 @@
 	if (isnull(outcome) && isnull(expected_reason))
 		return
 
-	if (isnull(outcome))
-		Fail("[message] was not blocked on the [filter_type] filter when it was expected to")
-		return
+	TEST_ASSERT_NOTNULL(outcome,
+		"[message] was not blocked on the [filter_type] filter when it was expected to")
 
-	if (isnull(expected_reason))
-		Fail("[message] was blocked on the [filter_type] filter when it wasn't expected to: [json_encode(outcome)]")
-		return
+	TEST_ASSERT_NOTNULL(expected_reason,
+		"[message] was blocked on the [filter_type] filter when it wasn't expected to: [json_encode(outcome)]")
 
-	if (outcome[CHAT_FILTER_INDEX_WORD] != expected_blocked_word)
-		Fail("[message] was blocked on the [filter_type] filter, but for a different word: \"[outcome[CHAT_FILTER_INDEX_WORD]]\" (instead of [expected_blocked_word])")
-		return
+	TEST_ASSERT_EQUAL(outcome[CHAT_FILTER_INDEX_WORD], expected_blocked_word,
+		"[message] was blocked on the [filter_type] filter, but for a different word: \"[outcome[CHAT_FILTER_INDEX_WORD]]\" (instead of [expected_blocked_word])")
 
-	if (outcome[CHAT_FILTER_INDEX_REASON] != expected_reason)
-		Fail("[message] was blocked on the [filter_type] filter, but for a different reason: \"[outcome[CHAT_FILTER_INDEX_REASON]]\" (instead of [expected_reason])")
+	TEST_ASSERT_EQUAL(outcome[CHAT_FILTER_INDEX_REASON], expected_reason,
+		"[message] was blocked on the [filter_type] filter, but for a different reason: \"[outcome[CHAT_FILTER_INDEX_REASON]]\" (instead of [expected_reason])")
 
 #undef BLOCKED_IC
 #undef BLOCKED_IC_OUTSIDE_PDA

code/modules/unit_tests/component_tests.dm

@@ -8,5 +8,5 @@
 		var/dupe_type = initial(comp.dupe_type)
 		if(dupe_type && !ispath(dupe_type))
 			bad_dts += t
-	if(length(bad_dms) || length(bad_dts))
-		Fail("Components with invalid dupe modes: ([bad_dms.Join(",")]) ||| Components with invalid dupe types: ([bad_dts.Join(",")])")
+	TEST_ASSERT(!length(bad_dms) && !length(bad_dts),
+		"Components with invalid dupe modes: ([bad_dms.Join(",")]) ||| Components with invalid dupe types: ([bad_dts.Join(",")])")

code/modules/unit_tests/crayons.dm

@@ -7,5 +7,5 @@
 			continue
 		var/obj/item/toy/crayon/real_crayon = new crayon_path
 		if(!findtext(initial(real_crayon.name),real_crayon.crayon_color))
-			Fail("[real_crayon] does not have its crayon_color ([real_crayon.crayon_color]) in its initial name ([initial(real_crayon.name)]).")
+			TEST_FAIL("[real_crayon] does not have its crayon_color ([real_crayon.crayon_color]) in its initial name ([initial(real_crayon.name)]).")
 		qdel(real_crayon)

code/modules/unit_tests/create_and_destroy.dm

@@ -98,7 +98,7 @@
 			//We change it back to prevent pain, please don't ask
 			spawn_at.ChangeTurf(/turf/open/floor/wood, /turf/baseturf_skipover)
 			if(baseturf_count != length(spawn_at.baseturfs))
-				Fail("[type_path] changed the amount of baseturfs we have [baseturf_count] -> [length(spawn_at.baseturfs)]")
+				TEST_FAIL("[type_path] changed the amount of baseturfs we have [baseturf_count] -> [length(spawn_at.baseturfs)]")
 				baseturf_count = length(spawn_at.baseturfs)
 		else
 			var/atom/creation = new type_path(spawn_at)
@@ -145,7 +145,7 @@
 			break
 
 		if(world.time > start_time + time_needed + 30 MINUTES) //If this gets us gitbanned I'm going to laugh so hard
-			Fail("Something has gone horribly wrong, the garbage queue has been processing for well over 30 minutes. What the hell did you do")
+			TEST_FAIL("Something has gone horribly wrong, the garbage queue has been processing for well over 30 minutes. What the hell did you do")
 			break
 
 		//Immediately fire the gc right after
@@ -158,21 +158,21 @@
 	for(var/path in cache_for_sonic_speed)
 		var/datum/qdel_item/item = cache_for_sonic_speed[path]
 		if(item.failures)
-			Fail("[item.name] hard deleted [item.failures] times out of a total del count of [item.qdels]")
+			TEST_FAIL("[item.name] hard deleted [item.failures] times out of a total del count of [item.qdels]")
 		if(item.no_respect_force)
-			Fail("[item.name] failed to respect force deletion [item.no_respect_force] times out of a total del count of [item.qdels]")
+			TEST_FAIL("[item.name] failed to respect force deletion [item.no_respect_force] times out of a total del count of [item.qdels]")
 		if(item.no_hint)
-			Fail("[item.name] failed to return a qdel hint [item.no_hint] times out of a total del count of [item.qdels]")
+			TEST_FAIL("[item.name] failed to return a qdel hint [item.no_hint] times out of a total del count of [item.qdels]")
 
 	cache_for_sonic_speed = SSatoms.BadInitializeCalls
 	for(var/path in cache_for_sonic_speed)
 		var/fails = cache_for_sonic_speed[path]
 		if(fails & BAD_INIT_NO_HINT)
-			Fail("[path] didn't return an Initialize hint")
+			TEST_FAIL("[path] didn't return an Initialize hint")
 		if(fails & BAD_INIT_QDEL_BEFORE)
-			Fail("[path] qdel'd in New()")
+			TEST_FAIL("[path] qdel'd in New()")
 		if(fails & BAD_INIT_SLEPT)
-			Fail("[path] slept during Initialize()")
+			TEST_FAIL("[path] slept during Initialize()")
 
 	SSticker.delay_end = FALSE
 	//This shouldn't be needed, but let's be polite

code/modules/unit_tests/designs.dm

@@ -12,21 +12,21 @@
 		if (current_design.id == DESIGN_ID_IGNORE) //Don't check designs with ignore ID
 			continue
 		if (isnull(current_design.name) || current_design.name == default_design.name) //Designs with ID must have non default/null Name
-			Fail("Design [current_design.type] has default or null name var but has an ID")
+			TEST_FAIL("Design [current_design.type] has default or null name var but has an ID")
 		if ((!isnull(current_design.materials) && LAZYLEN(current_design.materials)) || (!isnull(current_design.reagents_list) && LAZYLEN(current_design.reagents_list))) //Design requires materials
 			if ((isnull(current_design.build_path) || current_design.build_path == default_design.build_path) && (isnull(current_design.make_reagents) || current_design.make_reagents == default_design.make_reagents)) //Check if design gives any output
-				Fail("Design [current_design.type] requires materials but does not have have any build_path or make_reagents set")
+				TEST_FAIL("Design [current_design.type] requires materials but does not have have any build_path or make_reagents set")
 		else if (!isnull(current_design.build_path) || !isnull(current_design.build_path)) // //Design requires no materials but creates stuff
-			Fail("Design [current_design.type] requires NO materials but has build_path or make_reagents set")
+			TEST_FAIL("Design [current_design.type] requires NO materials but has build_path or make_reagents set")
 
 	for(var/path in subtypesof(/datum/design/surgery))
 		var/datum/design/surgery/current_design = new path //Create an instance of each design
 		if (isnull(current_design.id) || current_design.id == default_design_surgery.id) //Check if ID was not set
-			Fail("Surgery Design [current_design.type] has no ID set")
+			TEST_FAIL("Surgery Design [current_design.type] has no ID set")
 		if (isnull(current_design.id) || current_design.name == default_design_surgery.name) //Check if name was not set
-			Fail("Surgery Design [current_design.type] has default or null name var")
+			TEST_FAIL("Surgery Design [current_design.type] has default or null name var")
 		if (isnull(current_design.desc) || current_design.desc == default_design_surgery.desc) //Check if desc was not set
-			Fail("Surgery Design [current_design.type] has default or null desc var")
+			TEST_FAIL("Surgery Design [current_design.type] has default or null desc var")
 		if (isnull(current_design.surgery) || current_design.surgery == default_design_surgery.surgery) //Check if surgery was not set
-			Fail("Surgery Design [current_design.type] has default or null surgery var")
+			TEST_FAIL("Surgery Design [current_design.type] has default or null surgery var")
 

code/modules/unit_tests/dynamic_ruleset_sanity.dm

@@ -9,9 +9,9 @@
 		var/is_lone = initial(ruleset.flags) & (LONE_RULESET | HIGH_IMPACT_RULESET)
 
 		if (has_scaling_cost && is_lone)
-			Fail("[ruleset] has a scaling_cost, but is also a lone/highlander ruleset.")
+			TEST_FAIL("[ruleset] has a scaling_cost, but is also a lone/highlander ruleset.")
 		else if (!has_scaling_cost && !is_lone)
-			Fail("[ruleset] has no scaling cost, but is also not a lone/highlander ruleset.")
+			TEST_FAIL("[ruleset] has no scaling cost, but is also not a lone/highlander ruleset.")
 
 /// Verifies that dynamic rulesets have unique antag_flag.
 /datum/unit_test/dynamic_unique_antag_flags
@@ -26,11 +26,11 @@
 		var/antag_flag = initial(ruleset.antag_flag)
 
 		if (isnull(antag_flag))
-			Fail("[ruleset] has a null antag_flag!")
+			TEST_FAIL("[ruleset] has a null antag_flag!")
 			continue
 
 		if (antag_flag in known_antag_flags)
-			Fail("[ruleset] has a non-unique antag_flag [antag_flag] (used by [known_antag_flags[antag_flag]])!")
+			TEST_FAIL("[ruleset] has a non-unique antag_flag [antag_flag] (used by [known_antag_flags[antag_flag]])!")
 			continue
 
 		known_antag_flags[antag_flag] = ruleset

code/modules/unit_tests/egg_glands.dm

@@ -11,4 +11,4 @@
 		try
 			mix_color_from_reagents(egg.reagents.reagent_list + list(new reagent_type))
 		catch (var/exception/exception)
-			Fail("[reagent_type] fails mixing\n[exception]")
+			TEST_FAIL("[reagent_type] fails mixing\n[exception]")

code/modules/unit_tests/food_edibility_check.dm

@@ -16,9 +16,9 @@
 		var/obj/item/food/spawned_food = allocate(food_path)
 
 		if(!spawned_food.reagents)
-			Fail("[food_path] does not have any reagents, making it inedible!")
+			TEST_FAIL("[food_path] does not have any reagents, making it inedible!")
 
 		if(!IS_EDIBLE(spawned_food))
-			Fail("[food_path] does not have the edible component, making it inedible!")
+			TEST_FAIL("[food_path] does not have the edible component, making it inedible!")
 
 		qdel(spawned_food)

code/modules/unit_tests/greyscale_config.dm

@@ -7,21 +7,21 @@
 
 		var/datum/greyscale_config/lefthand = SSgreyscale.configurations["[initial(item_path.greyscale_config_inhand_left)]"]
 		if(lefthand && !lefthand.icon_states[held_icon_state])
-			Fail("[lefthand.DebugName()] is missing a sprite for the held lefthand for [item_path]. Expected icon state: '[held_icon_state]'")
+			TEST_FAIL("[lefthand.DebugName()] is missing a sprite for the held lefthand for [item_path]. Expected icon state: '[held_icon_state]'")
 
 		var/datum/greyscale_config/righthand = SSgreyscale.configurations["[initial(item_path.greyscale_config_inhand_right)]"]
 		if(righthand && !righthand.icon_states[held_icon_state])
-			Fail("[righthand.DebugName()] is missing a sprite for the held righthand for [item_path]. Expected icon state: '[held_icon_state]'")
+			TEST_FAIL("[righthand.DebugName()] is missing a sprite for the held righthand for [item_path]. Expected icon state: '[held_icon_state]'")
 
 		var/datum/greyscale_config/worn = SSgreyscale.configurations["[initial(item_path.greyscale_config_worn)]"]
 		var/worn_icon_state = initial(item_path.worn_icon_state) || initial(item_path.icon_state)
 		if(worn && !worn.icon_states[worn_icon_state])
-			Fail("[worn.DebugName()] is missing a sprite for the worn overlay for [item_path]. Expected icon state: '[worn_icon_state]'")
+			TEST_FAIL("[worn.DebugName()] is missing a sprite for the worn overlay for [item_path]. Expected icon state: '[worn_icon_state]'")
 
 		var/datum/greyscale_config/belt = SSgreyscale.configurations["[initial(item_path.greyscale_config_belt)]"]
 		var/belt_icon_state = initial(item_path.belt_icon_state) || initial(item_path.icon_state)
 		if(belt && !belt.icon_states[belt_icon_state])
-			Fail("[belt.DebugName()] is missing a sprite for the belt overlay for [item_path]. Expected icon state: '[belt_icon_state]'")
+			TEST_FAIL("[belt.DebugName()] is missing a sprite for the belt overlay for [item_path]. Expected icon state: '[belt_icon_state]'")
 
 /// Makes sure objects using greyscale configs have, if any, the correct number of colors
 /datum/unit_test/greyscale_color_count
@@ -36,4 +36,4 @@
 			continue
 		var/number_of_colors = length(colors) - 1
 		if(config.expected_colors != number_of_colors)
-			Fail("[thing] has the wrong amount of colors configured for [config.DebugName()]. Expected [config.expected_colors] but only found [number_of_colors].")
+			TEST_FAIL("[thing] has the wrong amount of colors configured for [config.DebugName()]. Expected [config.expected_colors] but only found [number_of_colors].")

code/modules/unit_tests/heretic_knowledge.dm

@@ -24,12 +24,12 @@
 	while(i < length(list_to_check))
 		var/datum/heretic_knowledge/path_to_create = list_to_check[++i]
 		if(!ispath(path_to_create))
-			Fail("Heretic Knowlege: Got a non-heretic knowledge datum (Got: [path_to_create]) in the list knowledges!")
+			TEST_FAIL("Heretic Knowlege: Got a non-heretic knowledge datum (Got: [path_to_create]) in the list knowledges!")
 		var/datum/heretic_knowledge/instantiated_knowledge = new path_to_create()
 		// Next knowledge is a list of typepaths.
 		for(var/datum/heretic_knowledge/next_knowledge as anything in instantiated_knowledge.next_knowledge)
 			if(!ispath(next_knowledge))
-				Fail("Heretic Knowlege: [next_knowledge.type] has a [isnull(next_knowledge) ? "null":"invalid path"] in its next_knowledge list!")
+				TEST_FAIL("Heretic Knowlege: [next_knowledge.type] has a [isnull(next_knowledge) ? "null":"invalid path"] in its next_knowledge list!")
 				continue
 			if(next_knowledge in list_to_check)
 				continue
@@ -44,7 +44,7 @@
 		// Unreachables is a list of typepaths - all paths that cannot be obtained.
 		var/list/unreachables = all_possible_knowledge - list_to_check
 		for(var/datum/heretic_knowledge/lost_knowledge as anything in unreachables)
-			Fail("Heretic Knowlege: [lost_knowledge] is unreachable by players! Add it to another knowledge's 'next_knowledge' list. If it is purposeful, set its route to 'null'.")
+			TEST_FAIL("Heretic Knowlege: [lost_knowledge] is unreachable by players! Add it to another knowledge's 'next_knowledge' list. If it is purposeful, set its route to 'null'.")
 
 
 /*
@@ -77,7 +77,7 @@
 			continue
 
 		if(isnull(paths[knowledge_route]))
-			Fail("Heretic Knowledge: An invalid knowledge route ([knowledge_route]) was found on [knowledge].")
+			TEST_FAIL("Heretic Knowledge: An invalid knowledge route ([knowledge_route]) was found on [knowledge].")
 			continue
 
 		paths[knowledge_route]++

code/modules/unit_tests/heretic_rituals.dm

@@ -78,7 +78,7 @@
 				qdel(leftover)
 
 			// Aaand throw a fail.
-			Fail("Heretic rituals: ([knowledge.type]) Despite having all required atoms present, the ritual failed to transmute.")
+			TEST_FAIL("Heretic rituals: ([knowledge.type]) Despite having all required atoms present, the ritual failed to transmute.")
 			continue
 
 		// Making it here means the ritual was a success.
@@ -92,7 +92,7 @@
 			var/atom/result = locate(result_item_path) in nearby_atoms
 			// No, we couldn't find the a resulting atom on the rune. Throw a fail.
 			if(!result)
-				Fail("Heretic rituals: ([knowledge.type]) Despite successfully completing the ritual, a resulting atom could not be found ([result_item_path])")
+				TEST_FAIL("Heretic rituals: ([knowledge.type]) Despite successfully completing the ritual, a resulting atom could not be found ([result_item_path])")
 				continue
 
 			// Yes, we got a resulting atom we expected! Remove it from the list and clean up.
@@ -108,7 +108,7 @@
 			// There are atoms around the rune still, and there shouldn't be.
 			// All component atoms were consumed, and all resulting atoms were cleaned up.
 			// This means the ritual may have messed up somewhere. Throw a fail and clean them up so we can keep testing.
-			Fail("Heretic rituals: ([knowledge.type]) After completing the ritual, there were non-result atoms remaining on the rune. ([thing] - [thing.type])")
+			TEST_FAIL("Heretic rituals: ([knowledge.type]) After completing the ritual, there were non-result atoms remaining on the rune. ([thing] - [thing.type])")
 			nearby_atoms -= thing
 			qdel(thing)
 

code/modules/unit_tests/hydroponics_harvest.dm

@@ -56,10 +56,10 @@
 	var/saved_name = tray.name // Name gets cleared when some plants are harvested.
 
 	if(!tray.myseed)
-		Fail("Hydroponics harvest from [saved_name] had no seed set properly to test.")
+		TEST_FAIL("Hydroponics harvest from [saved_name] had no seed set properly to test.")
 
 	if(tray.myseed != seed)
-		Fail("Hydroponics harvest from [saved_name] had [tray.myseed] planted when it was testing [seed].")
+		TEST_FAIL("Hydroponics harvest from [saved_name] had [tray.myseed] planted when it was testing [seed].")
 
 	var/double_chemicals = seed.get_gene(/datum/plant_gene/trait/maxchem)
 	var/expected_yield = seed.getYield()
@@ -74,7 +74,7 @@
 		all_harvested_items += harvested_food
 
 	if(!all_harvested_items.len)
-		Fail("Hydroponics harvest from [saved_name] resulted in 0 harvest.")
+		TEST_FAIL("Hydroponics harvest from [saved_name] resulted in 0 harvest.")
 
 	TEST_ASSERT_EQUAL(all_harvested_items.len, expected_yield, "Hydroponics harvest from [saved_name] only harvested [all_harvested_items.len] items instead of [expected_yield] items.")
 	TEST_ASSERT(all_harvested_items[1].reagents, "Hydroponics harvest from [saved_name] had no reagent container.")

code/modules/unit_tests/keybinding_init.dm

@@ -3,4 +3,4 @@
 		var/datum/keybinding/KB = i
 		if(initial(KB.keybind_signal) || !initial(KB.name))
 			continue
-		Fail("[KB.name] does not have a keybind signal defined.")
+		TEST_FAIL("[KB.name] does not have a keybind signal defined.")

code/modules/unit_tests/merge_type.dm

@@ -12,4 +12,4 @@
 	for(var/stackpath in paths)
 		var/obj/item/stack/stack = stackpath
 		if(!initial(stack.merge_type))
-			Fail("([stack]) lacks set merge_type variable!")
+			TEST_FAIL("([stack]) lacks set merge_type variable!")

code/modules/unit_tests/mob_spawn.dm

@@ -17,7 +17,7 @@
 		var/obj/effect/mob_spawn/ghost_role/ghost_role = allocate(role_spawn_path)
 
 		if(ghost_role.outfit_override)
-			Fail("[ghost_role.type] has a defined \"outfit_override\" list, which is only for mapping. Do not set this!")
+			TEST_FAIL("[ghost_role.type] has a defined \"outfit_override\" list, which is only for mapping. Do not set this!")
 
 		if(ghost_role.mob_type != /mob/living/carbon/human)
 			//vars that must not be set if the mob type isn't human
@@ -32,7 +32,7 @@
 			)
 			for(var/human_only_var in human_only_vars)
 				if(ghost_role.vars[human_only_var])
-					Fail("[ghost_role.type] has a defined \"[human_only_var]\" HUMAN ONLY var, but this type doesn't spawn humans.")
+					TEST_FAIL("[ghost_role.type] has a defined \"[human_only_var]\" HUMAN ONLY var, but this type doesn't spawn humans.")
 
 		//vars that must be set on
 		var/list/required_vars = list(
@@ -46,6 +46,6 @@
 			if(required_var == "prompt_name" && !ghost_role.prompt_ghost)
 				continue //only case it makes sense why you shouldn't have a prompt_name
 			if(!ghost_role.vars[required_var])
-				Fail("[ghost_role.type] must have \"[required_var]\" defined. Reason: [required_vars[required_var]]")
+				TEST_FAIL("[ghost_role.type] must have \"[required_var]\" defined. Reason: [required_vars[required_var]]")
 
 		qdel(ghost_role)

code/modules/unit_tests/modular_map_loader.dm

@@ -4,7 +4,7 @@
 	for (var/obj/modular_map_root/map_root_type as anything in subtypesof(/obj/modular_map_root))
 		var/config_file = initial(map_root_type.config_file)
 		if (!fexists(config_file))
-			Fail("[map_root_type] points to a config file which does not exist!")
+			TEST_FAIL("[map_root_type] points to a config file which does not exist!")
 			continue
 		if (rustg_read_toml_file(config_file) == null)
-			Fail("[map_root_type] points to a config file which is invalid!")
+			TEST_FAIL("[map_root_type] points to a config file which is invalid!")

code/modules/unit_tests/objectives.dm

@@ -13,7 +13,7 @@
 		if(initial(objective_typepath.abstract_type) == objective_typepath)
 			continue
 		if(!(objective_typepath in objectives_that_exist))
-			Fail("[objective_typepath] is not in a traitor category and isn't an abstract type! Place it into a [/datum/traitor_objective_category] or remove it from code.")
+			TEST_FAIL("[objective_typepath] is not in a traitor category and isn't an abstract type! Place it into a [/datum/traitor_objective_category] or remove it from code.")
 
 /datum/unit_test/objectives_category/proc/recursive_check_list(base_type, list/to_check, list/to_add_to)
 	for(var/value in to_check)

code/modules/unit_tests/outfit_sanity.dm

@@ -3,7 +3,7 @@
 	/* We don't check the result of equip_to_slot_or_del because it returns false for random jumpsuits, as they delete themselves on init */ \
 	var/obj/item/outfit_item = H.get_item_by_slot(##slot_name); \
 	if (!outfit_item) { \
-		Fail("[outfit.name]'s [#outfit_key] is invalid! Could not equip a [outfit.##outfit_key] into that slot."); \
+		TEST_FAIL("[outfit.name]'s [#outfit_key] is invalid! Could not equip a [outfit.##outfit_key] into that slot."); \
 	} \
 	outfit_item.on_outfit_equip(H, FALSE, ##slot_name); \
 }
@@ -57,6 +57,6 @@
 				var/number = backpack_contents[path] || 1
 				for (var/_ in 1 to number)
 					if (!H.equip_to_slot_or_del(new path(H), ITEM_SLOT_BACKPACK, TRUE))
-						Fail("[outfit.name]'s backpack_contents are invalid! Couldn't add [path] to backpack.")
+						TEST_FAIL("[outfit.name]'s backpack_contents are invalid! Couldn't add [path] to backpack.")
 
 #undef CHECK_OUTFIT_SLOT

code/modules/unit_tests/paintings.dm

@@ -7,4 +7,4 @@
 		var/canvas_icons = icon_states(canvas.icon)
 		for(var/frame_type in SSpersistent_paintings.frame_types_by_patronage_tier)
 			if(!("[canvas.icon_state]frame_[frame_type]" in canvas_icons))
-				Fail("Canvas [canvas.icon_state] doesn't have an icon state for frame: [frame_type].")
+				TEST_FAIL("Canvas [canvas.icon_state] doesn't have an icon state for frame: [frame_type].")

code/modules/unit_tests/plantgrowth_tests.dm

@@ -16,11 +16,11 @@
 		for(var/i in 1 to seed.growthstages)
 			if("[seed.icon_grow][i]" in states)
 				continue
-			Fail("[seed.name] ([seed.type]) lacks the [seed.icon_grow][i] icon!")
+			TEST_FAIL("[seed.name] ([seed.type]) lacks the [seed.icon_grow][i] icon!")
 
 		if(!(seed.icon_dead in states))
-			Fail("[seed.name] ([seed.type]) lacks the [seed.icon_dead] icon!")
+			TEST_FAIL("[seed.name] ([seed.type]) lacks the [seed.icon_dead] icon!")
 
 		if(seed.icon_harvest) // mushrooms have no grown sprites, same for items with no product
 			if(!(seed.icon_harvest in states))
-				Fail("[seed.name] ([seed.type]) lacks the [seed.icon_harvest] icon!")
+				TEST_FAIL("[seed.name] ([seed.type]) lacks the [seed.icon_harvest] icon!")

code/modules/unit_tests/preference_species.dm

@@ -19,15 +19,15 @@
 		// If it's null, it was improperly overriden. Fail the test.
 		var/species_desc = species.get_species_description()
 		if(isnull(species_desc))
-			Fail("Species [species] ([species_type]) is selectable, but did not properly implement get_species_description().")
+			TEST_FAIL("Species [species] ([species_type]) is selectable, but did not properly implement get_species_description().")
 
 		// Check the species lore.
 		// If it's not overridden, a stack trace will be thrown (and fail the test).
 		// If it's null, or returned a list, it was improperly overriden. Fail the test.
 		var/species_lore = species.get_species_lore()
 		if(isnull(species_lore))
-			Fail("Species [species] ([species_type]) is selectable, but did not properly implement get_species_lore().")
+			TEST_FAIL("Species [species] ([species_type]) is selectable, but did not properly implement get_species_lore().")
 		else if(!islist(species_lore))
-			Fail("Species [species] ([species_type]) is selectable, but did not properly implement get_species_lore() (Did not return a list).")
+			TEST_FAIL("Species [species] ([species_type]) is selectable, but did not properly implement get_species_lore() (Did not return a list).")
 
 		qdel(species)

code/modules/unit_tests/preferences.dm

@@ -29,10 +29,10 @@
 	for (var/preference_type in GLOB.preference_entries)
 		var/datum/preference/preference = GLOB.preference_entries[preference_type]
 		if (!istext(preference.savefile_key))
-			Fail("[preference_type] has an invalid savefile_key.")
+			TEST_FAIL("[preference_type] has an invalid savefile_key.")
 
 		if (preference.savefile_key in known_savefile_keys)
-			Fail("[preference_type] has a non-unique savefile_key `[preference.savefile_key]`!")
+			TEST_FAIL("[preference_type] has a non-unique savefile_key `[preference.savefile_key]`!")
 
 		known_savefile_keys += preference.savefile_key
 

code/modules/unit_tests/projectiles.dm

@@ -2,7 +2,7 @@
 	for(var/path in typesof(/obj/projectile))
 		var/obj/projectile/projectile = path
 		if(initial(projectile.movement_type) & PHASING)
-			Fail("[path] has default movement type PHASING. Piercing projectiles should be done using the projectile piercing system, not movement_types!")
+			TEST_FAIL("[path] has default movement type PHASING. Piercing projectiles should be done using the projectile piercing system, not movement_types!")
 
 /datum/unit_test/gun_go_bang/Run()
 	// test is for a ballistic gun that starts loaded + chambered

code/modules/unit_tests/quirks.dm

@@ -11,11 +11,11 @@
 		var/icon = initial(quirk_type.icon)
 
 		if (isnull(icon))
-			Fail("[quirk_type] has no icon!")
+			TEST_FAIL("[quirk_type] has no icon!")
 			continue
 
 		if (icon in used_icons)
-			Fail("[icon] used in both [quirk_type] and [used_icons[icon]]!")
+			TEST_FAIL("[icon] used in both [quirk_type] and [used_icons[icon]]!")
 			continue
 
 		used_icons[icon] = quirk_type

code/modules/unit_tests/rcd.dm

@@ -19,8 +19,7 @@
 
 	var/list/adjacent_turfs = get_adjacent_open_turfs(engineer)
 
-	if(!length(adjacent_turfs))
-		Fail("RCD Test failed - Lack of adjacent open turfs. This may be an issue with the unit test.")
+	TEST_ASSERT(length(adjacent_turfs), "RCD Test failed - Lack of adjacent open turfs. This may be an issue with the unit test.")
 
 	var/turf/adjacent_turf = adjacent_turfs[1]
 

code/modules/unit_tests/reagent_id_typos.dm

@@ -10,4 +10,4 @@
 			var/datum/chemical_reaction/R = V
 			for(var/id in (R.required_reagents + R.required_catalysts))
 				if(!GLOB.chemical_reagents_list[id])
-					Fail("Unknown chemical id \"[id]\" in recipe [R.type]")
+					TEST_FAIL("Unknown chemical id \"[id]\" in recipe [R.type]")

code/modules/unit_tests/reagent_names.dm

@@ -10,6 +10,6 @@
 			continue
 
 		if (name in used_names)
-			Fail("[used_names[name]] shares a name with [reagent] ([name])")
+			TEST_FAIL("[used_names[name]] shares a name with [reagent] ([name])")
 		else
 			used_names[name] = reagent

code/modules/unit_tests/reagent_recipe_collisions.dm

@@ -12,4 +12,4 @@
 			var/datum/chemical_reaction/r1 = reactions[i]
 			var/datum/chemical_reaction/r2 = reactions[i2]
 			if(chem_recipes_do_conflict(r1, r2))
-				Fail("Chemical recipe conflict between [r1.type] and [r2.type]")
+				TEST_FAIL("Chemical recipe conflict between [r1.type] and [r2.type]")

code/modules/unit_tests/say.dm

@@ -19,5 +19,5 @@
 		TEST_ASSERT_EQUAL(mods[mod_key], expected_mods[mod_key], "The value for [mod_key] was not what we expected. Message: [message]")
 		expected_mods -= mod_key
 
-	if (expected_mods.len)
-		Fail("Some message mods were expected, but were not returned by get_message_mods: [json_encode(expected_mods)]. Message: [message]")
+	TEST_ASSERT(!expected_mods.len,
+		"Some message mods were expected, but were not returned by get_message_mods: [json_encode(expected_mods)]. Message: [message]")

code/modules/unit_tests/security_officer_distribution.dm

@@ -18,11 +18,9 @@
 
 	if (outcome.len == expected.len)
 		for (var/index in 1 to outcome.len)
-			if (outcome[index] != expected[index])
-				Fail(failure_message)
-				return
+			TEST_ASSERT_EQUAL(outcome[index], expected[index], failure_message)
 	else
-		Fail(failure_message)
+		TEST_FAIL(failure_message)
 
 /datum/unit_test/security_officer_roundstart_distribution/Run()
 	test_distributions()

code/modules/unit_tests/species_config_sanity.dm

@@ -17,6 +17,6 @@
 	for(var/datum/species/species_type as anything in subtypesof(/datum/species))
 		var/species_id = initial(species_type.id)
 		if(findtext(species_id, first_splitter))
-			Fail("A species ID contained a config_entry splitter: [species_type] | Splitter: (\"[first_splitter]\") | Species ID: (\"[species_id]\")")
+			TEST_FAIL("A species ID contained a config_entry splitter: [species_type] | Splitter: (\"[first_splitter]\") | Species ID: (\"[species_id]\")")
 		if(findtext(species_id, second_splitter))
-			Fail("A species ID contained a config_entry splitter: [species_type] | Splitter: (\"[second_splitter]\") | Species ID: (\"[species_id]\")")
+			TEST_FAIL("A species ID contained a config_entry splitter: [species_type] | Splitter: (\"[second_splitter]\") | Species ID: (\"[species_id]\")")

code/modules/unit_tests/species_unique_id.dm

@@ -9,6 +9,6 @@
 	for(var/datum/species/species as anything in subtypesof(/datum/species))
 		var/species_id = initial(species.id)
 		if(gathered_species_ids[species_id])
-			Fail("Duplicate species ID! [species_id] is not unique to a single species.")
+			TEST_FAIL("Duplicate species ID! [species_id] is not unique to a single species.")
 		else
 			gathered_species_ids[species_id] = TRUE

code/modules/unit_tests/species_whitelists.dm

@@ -2,4 +2,4 @@
 	for(var/typepath in subtypesof(/datum/species))
 		var/datum/species/S = typepath
 		if(initial(S.changesource_flags) == NONE)
-			Fail("A species type was detected with no changesource flags: [S]")
+			TEST_FAIL("A species type was detected with no changesource flags: [S]")

code/modules/unit_tests/stack_singular_name.dm

@@ -15,4 +15,4 @@
 
 	for(var/obj/item/stack/stack_check as anything in subtypesof(/obj/item/stack) - blacklist)
 		if(!initial(stack_check.singular_name))
-			Fail("[stack_check] is missing a singular name!")
+			TEST_FAIL("[stack_check] is missing a singular name!")

code/modules/unit_tests/subsystem_init.dm

@@ -4,4 +4,4 @@
 		if(ss.flags & SS_NO_INIT)
 			continue
 		if(!ss.initialized)
-			Fail("[ss]([ss.type]) is a subsystem meant to initialize but doesn't get set as initialized.")
+			TEST_FAIL("[ss]([ss.type]) is a subsystem meant to initialize but doesn't get set as initialized.")

code/modules/unit_tests/surgeries.dm

@@ -87,7 +87,7 @@
 	var/datum/surgery/surgery = new /datum/surgery/healing/brute/basic
 
 	if (!surgery.can_start(user, patient))
-		Fail("Can't start basic tend wounds!")
+		TEST_FAIL("Can't start basic tend wounds!")
 
 	qdel(surgery)
 

code/modules/unit_tests/timer_sanity.dm

@@ -1,3 +1,3 @@
 /datum/unit_test/timer_sanity/Run()
-	if(SStimer.bucket_count < 0)
-		Fail("SStimer is going into negative bucket count from something")
+	TEST_ASSERT(SStimer.bucket_count >= 0,
+		"SStimer is going into negative bucket count from something")

code/modules/unit_tests/traitor.dm

@@ -21,7 +21,7 @@
 		mind.set_assigned_role(job)
 		var/datum/antagonist/traitor/traitor = mind.add_antag_datum(/datum/antagonist/traitor)
 		if(!traitor.uplink_handler)
-			Fail("[job_name] when made traitor does not have a proper uplink created when spawned in!")
+			TEST_FAIL("[job_name] when made traitor does not have a proper uplink created when spawned in!")
 		for(var/datum/traitor_objective/objective_typepath as anything in subtypesof(/datum/traitor_objective))
 			if(initial(objective_typepath.abstract_type) == objective_typepath)
 				continue
@@ -29,4 +29,4 @@
 			try
 				objective.generate_objective(mind, list())
 			catch(var/exception/exception)
-				Fail("[objective_typepath] failed to generate their objective. Reason: [exception.name] [exception.file]:[exception.line]\n[exception.desc]")
+				TEST_FAIL("[objective_typepath] failed to generate their objective. Reason: [exception.name] [exception.file]:[exception.line]\n[exception.desc]")

code/modules/unit_tests/unit_test.dm

@@ -3,7 +3,7 @@
 Usage:
 Override /Run() to run your test code
 
-Call Fail() to fail the test (You should specify a reason)
+Call TEST_FAIL() to fail the test (You should specify a reason)
 
 You may use /New() and /Destroy() for setup/teardown respectively
 
@@ -60,15 +60,15 @@ GLOBAL_VAR(test_log)
 	return ..()
 
 /datum/unit_test/proc/Run()
-	Fail("Run() called parent or not implemented")
+	TEST_FAIL("Run() called parent or not implemented")
 
-/datum/unit_test/proc/Fail(reason = "No reason")
+/datum/unit_test/proc/Fail(reason = "No reason", file = "OUTDATED_TEST", line = 1)
 	succeeded = FALSE
 
 	if(!istext(reason))
 		reason = "FORMATTED: [reason != null ? reason : "NULL"]"
 
-	LAZYADD(fail_reasons, reason)
+	LAZYADD(fail_reasons, list(list(reason, file, line)))
 
 /// Allocates an instance of the provided type, and places it somewhere in an available loc
 /// Instances allocated through this proc will be destroyed when the test is over
@@ -94,11 +94,22 @@ GLOBAL_VAR(test_log)
 	GLOB.current_test = null
 	GLOB.failed_any_test |= !test.succeeded
 
-	var/list/log_entry = list("[test.succeeded ? "PASS" : "FAIL"]: [test_path] [duration / 10]s")
+	var/list/log_entry = list(
+		"[test.succeeded ? TEST_OUTPUT_GREEN("PASS") : TEST_OUTPUT_RED("FAIL")]: [test_path] [duration / 10]s",
+	)
 	var/list/fail_reasons = test.fail_reasons
 
-	for(var/J in 1 to LAZYLEN(fail_reasons))
-		log_entry += "\tREASON #[J]: [fail_reasons[J]]"
+	for(var/reasonID in 1 to LAZYLEN(fail_reasons))
+		var/text = fail_reasons[reasonID][1]
+		var/file = fail_reasons[reasonID][2]
+		var/line = fail_reasons[reasonID][3]
+
+		/// Github action annotation.
+		log_world("::error file=[file],line=[line],title=[test_path]::[text]")
+
+		// Normal log message
+		log_entry += "\tREASON #[reasonID]: [text] at [file]:[line]"
+
 	var/message = log_entry.Join("\n")
 	log_test(message)