tweaks the syntax of unit tests to be caught by flaky tests

check_regex.yaml

@@ -38,7 +38,7 @@ standards:
 
     - exactly:
           [
-              261,
+              262,
               "non-bitwise << uses",
               '(?<!\d)(?<!\d\s)(?<!<)<<(?!=|\s\d|\d|<|\/)',
           ]

code/modules/antagonists/santa/santa.dm

@@ -17,7 +17,6 @@
 	to_chat(owner, "<span class='boldannounce'>You are Santa! Your objective is to bring joy to the people on this station. You have a magical bag, which generates presents as long as you have it! You can examine the presents to take a peek inside, to make sure that you give the right gift to the right person.</span>")
 
 /datum/antagonist/santa/proc/give_equipment()
-	var/mob/living/carbon/human/H = owner.current
 	owner.AddSpell(new /obj/effect/proc_holder/spell/targeted/area_teleport/teleport/santa)
 
 /datum/antagonist/santa/proc/give_objective()

code/modules/unit_tests/_unit_tests.dm

@@ -41,6 +41,11 @@
 /// Intended to be used in the manner of `TEST_FOCUS(/datum/unit_test/math)`
 #define TEST_FOCUS(test_path) ##test_path { focus = TRUE; }
 
+/// Logs a noticable message on GitHub, but will not mark as an error.
+/// Use this when something shouldn't happen and is of note, but shouldn't block CI.
+/// Does not mark the test as failed.
+#define TEST_NOTICE(source, message) source.log_for_test((##message), "notice", __FILE__, __LINE__)
+
 /// Constants indicating unit test completion status
 #define UNIT_TEST_PASSED 0
 #define UNIT_TEST_FAILED 1
@@ -81,7 +86,6 @@
 #include "keybinding_init.dm"
 #include "machine_disassembly.dm"
 #include "open_air.dm"
-#include "outfit_names.dm"
 #include "outfit_sanity.dm"
 #include "overmap.dm"
 #include "pills.dm"

code/modules/unit_tests/outfit_sanity.dm

@@ -6,7 +6,7 @@
 		if (outfit.random != TRUE) \
 			TEST_FAIL("[outfit.name]'s [#outfit_key] is invalid! Could not equip a [outfit.##outfit_key] into that slot."); \
 		else \
-			log_world("[outfit.name]'s [#outfit_key] is invalid! Could not equip a [outfit.##outfit_key] into that slot."); \
+			log_test("[outfit.name]'s [#outfit_key] is invalid! Could not equip a [outfit.##outfit_key] into that slot."); \
 	} \
 }
 
@@ -26,15 +26,20 @@
 	var/prototype_name = initial(prototype_outfit.name)
 	var/mob/living/carbon/human/H = allocate(/mob/living/carbon/human)
 
+	var/list/outfit_names = list()
 	for (var/outfit_type in subtypesof(/datum/outfit))
 		// Only make one human and keep undressing it because it's much faster
 		for (var/obj/item/I in H.get_equipped_items(include_pockets = TRUE))
 			qdel(I)
 
 		var/datum/outfit/outfit = new outfit_type
 
-		if(outfit.name == prototype_name)
+		var/outfit_name = outfit.name
+		if(outfit_name == prototype_name)
 			TEST_FAIL("[outfit.type]'s name is invalid! Uses default outfit name!")
+		if(outfit_name in outfit_names)
+			TEST_FAIL("Outfit name [outfit_name] is not unique: [outfit_type], [outfit_names[outfit_name]]")
+		outfit_names[outfit_name] = outfit_type
 		outfit.pre_equip(H, TRUE)
 
 		CHECK_OUTFIT_SLOT(uniform, ITEM_SLOT_ICLOTHING)
@@ -67,7 +72,7 @@
 						if (outfit.random != TRUE)
 							TEST_FAIL("[outfit.name]'s backpack_contents are invalid! Couldn't add [path] to backpack.")
 						else
-							log_world("[outfit.name]'s backpack_contents are invalid! Couldn't add [path] to backpack.")
+							log_test("[outfit.name]'s backpack_contents are invalid! Couldn't add [path] to backpack.")
 
 
 #undef CHECK_OUTFIT_SLOT

code/modules/unit_tests/ship_outpost_placement.dm

@@ -3,7 +3,7 @@
 	// disabled or intended as subshuttles
 	for(var/name as anything in SSmapping.shuttle_templates)
 		var/datum/map_template/shuttle/map = SSmapping.shuttle_templates[name]
-		log_world("Loading [map.name]")
+		log_test("Loading [map.name]")
 		try
 			// they'll spawn in empty space, and won't be docked
 			new /datum/overmap/ship/controlled(list("x" = 1, "y" = 1), map)
@@ -13,10 +13,10 @@
 	for(var/outpost_type in subtypesof(/datum/overmap/outpost))
 		var/datum/overmap/outpost/test_outpost = new outpost_type()
 
-		log_world("Testing [test_outpost.type]")
+		log_test("Testing [test_outpost.type]")
 
 		for(var/datum/overmap/ship/controlled/cur_ship as anything in SSovermap.controlled_ships)
-			log_world(" - Docking [cur_ship.source_template.name]")
+			log_test(" - Docking [cur_ship.source_template.name]")
 
 			// already-docked ships are ignored.
 			// this was added to stop runtimes when subshuttles, which were docked to their parent ship, attempted to dock to the outpost as part of this test.

code/modules/unit_tests/unit_test.dm

@@ -85,38 +85,58 @@ GLOBAL_VAR(test_log)
 	allocated += instance
 	return instance
 
+/// Logs a test message. Will use GitHub action syntax found at https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
+/datum/unit_test/proc/log_for_test(text, priority, file, line, test_path)
+	// Need to escape the text to properly support newlines.
+	var/annotation_text = replacetext(text, "%", "%25")
+	annotation_text = replacetext(annotation_text, "\n", "%0A")
+
+	log_world("::[priority] file=[file],line=[line],title=[test_path]: [type]::[annotation_text]")
+
 /proc/RunUnitTest(test_path, list/test_results)
 	var/datum/unit_test/test = new test_path
 
 	GLOB.current_test = test
 	var/duration = REALTIMEOFDAY
 
+	log_world("::group::[test_path]")
 	test.Run()
 
 	duration = REALTIMEOFDAY - duration
 	GLOB.current_test = null
 	GLOB.failed_any_test |= !test.succeeded
 
-	var/list/log_entry = list(
-		"[test.succeeded ? TEST_OUTPUT_GREEN("PASS") : TEST_OUTPUT_RED("FAIL")]: [test_path] [duration / 10]s",
-	)
+	var/list/log_entry = list()
 	var/list/fail_reasons = test.fail_reasons
 
+	var/test_output_desc = "[test_path]"
+	var/message = ""
+
 	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]")
+		test.log_for_test(text, "error", file, line, test_path)
 
 		// Normal log message
 		log_entry += "\tFAILURE #[reasonID]: [text] at [file]:[line]"
 
-	var/message = log_entry.Join("\n")
-	log_test(message)
+	if(length(log_entry))
+		message = log_entry.Join("\n")
+		log_test(message)
+
+	test_output_desc += " [duration / 10]s"
+	if (test.succeeded)
+		log_world("[TEST_OUTPUT_GREEN("PASS")] [test_output_desc]")
 
-	test_results[test_path] = list("status" = test.succeeded ? UNIT_TEST_PASSED : UNIT_TEST_FAILED, "message" = message, "name" = test_path)
+	log_world("::endgroup::")
+
+	if (!test.succeeded)
+		log_world("::error::[TEST_OUTPUT_RED("FAIL")] [test_output_desc]")
+
+	var/final_status = test.succeeded ? UNIT_TEST_PASSED : UNIT_TEST_FAILED
+	test_results[test_path] = list("status" = final_status, "message" = message, "name" = test_path)
 
 	qdel(test)
 
@@ -141,6 +161,10 @@ GLOBAL_VAR(test_log)
 		CHECK_TICK //We check tick first because the unit test we run last may be so expensive that checking tick will lock up this loop forever
 		RunUnitTest(unit_path, test_results)
 
+	var/file_name = "data/unit_tests.json"
+	fdel(file_name)
+	file(file_name) << json_encode(test_results)
+
 	SSticker.force_ending = TRUE
 	//We have to call this manually because del_text can preceed us, and SSticker doesn't fire in the post game
 	SSticker.declare_completion()