[MIRROR] Makes integration test results be in color and have annotati…

…ons (#13341)

.github/workflows/ci_suite.yml

@@ -98,11 +98,14 @@ jobs:
           sudo apt update || true
           sudo apt install -o APT::Immediate-Configure=false libssl1.1:i386
           bash tools/ci/install_rust_g.sh
-      - name: Compile and run tests
+      - name: Compile Tests
         run: |
           bash tools/ci/install_byond.sh
           source $HOME/BYOND/byond/bin/byondsetup
-          tools/build/build --ci dm -DCIBUILDING
+          tools/build/build --ci dm -DCIBUILDING -DANSICOLORS
+      - name: Run Tests
+        run: |
+          source $HOME/BYOND/byond/bin/byondsetup
           bash tools/ci/run_server.sh
 
   test_windows:

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,17 +3,26 @@
 
 #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
 #define TEST_ASSERT_EQUAL(a, b, message) do { \
 	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])")
-
-	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]")
-
-	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(alice.z, space_tile.destination_z, "Alice did not teleport to the destination z-level. Current location: ([alice.x], [alice.y], [alice.z])")
+
+	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]")
+
+	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

@@ -107,7 +107,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)
@@ -154,7 +154,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
@@ -167,21 +167,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)