Merge branch 'master' of https://github.com/odin-lang/Odin

2 files changed, 50 insertions, 19 deletions

diff --git a/core/testing/runner.odin b/core/testing/runner.odin
index 328186c35..6a33436f6 100644
--- a/core/testing/runner.odin
+++ b/core/testing/runner.odin
@@ -497,6 +497,7 @@ runner :: proc(internal_tests: []Internal_Test) -> bool {
 				data.it = it
 				data.t.seed = shared_random_seed
 				data.t.error_count = 0
+				data.t._fail_now_called = false
 
 				thread.pool_add_task(&pool, task.allocator, run_test_task, data, run_index)
 			}
@@ -604,10 +605,10 @@ runner :: proc(internal_tests: []Internal_Test) -> bool {
 			})
 			fmt.assertf(alloc_error == nil, "Error appending to log messages: %v", alloc_error)
 
-			find_task_data: for &data in task_data_slots {
+			find_task_data_for_timeout: for &data in task_data_slots {
 				if data.it.pkg == it.pkg && data.it.name == it.name {
 					end_t(&data.t)
-					break find_task_data
+					break find_task_data_for_timeout
 				}
 			}
 		}
@@ -645,21 +646,36 @@ runner :: proc(internal_tests: []Internal_Test) -> bool {
 				"A signal (%v) was raised to stop test #%i %s.%s, but it was unable to be found.",
 				reason, test_index, it.pkg, it.name)
 
-			if test_index not_in failed_test_reason_map {
-				// We only write a new error message here if there wasn't one
-				// already, because the message we can provide based only on
-				// the signal won't be very useful, whereas asserts and panics
-				// will provide a user-written error message.
-				failed_test_reason_map[test_index] = fmt.aprintf("Signal caught: %v", reason, allocator = shared_log_allocator)
-				pkg_log.fatalf("Caught signal to stop test #%i %s.%s for: %v.", test_index, it.pkg, it.name, reason)
-
+			// The order this is handled in is a little particular.
+			task_data: ^Task_Data
+			find_task_data_for_stop_signal: for &data in task_data_slots {
+				if data.it.pkg == it.pkg && data.it.name == it.name {
+					task_data = &data
+					break find_task_data_for_stop_signal
+				}
 			}
 
-			when FANCY_OUTPUT {
-				bypass_progress_overwrite = true
-				signals_were_raised = true
+			fmt.assertf(task_data != nil, "A signal (%v) was raised to stop test #%i %s.%s, but its task data is missing.",
+				reason, test_index, it.pkg, it.name)
+
+			if !task_data.t._fail_now_called {
+				if test_index not_in failed_test_reason_map {
+					// We only write a new error message here if there wasn't one
+					// already, because the message we can provide based only on
+					// the signal won't be very useful, whereas asserts and panics
+					// will provide a user-written error message.
+					failed_test_reason_map[test_index] = fmt.aprintf("Signal caught: %v", reason, allocator = shared_log_allocator)
+					pkg_log.fatalf("Caught signal to stop test #%i %s.%s for: %v.", test_index, it.pkg, it.name, reason)
+				}
+
+				when FANCY_OUTPUT {
+					bypass_progress_overwrite = true
+					signals_were_raised = true
+				}
 			}
 
+			end_t(&task_data.t)
+
 			total_failure_count += 1
 			total_done_count += 1
 		}
diff --git a/core/testing/testing.odin b/core/testing/testing.odin
index 92b4d391d..29fe853ef 100644
--- a/core/testing/testing.odin
+++ b/core/testing/testing.odin
@@ -48,7 +48,7 @@ T :: struct {
 	// tests during channel transmission.
 	_log_allocator: runtime.Allocator,
 
-	_fail_now: proc() -> !,
+	_fail_now_called: bool,
 }
 
 
@@ -66,15 +66,20 @@ fail :: proc(t: ^T, loc := #caller_location) {
 	pkg_log.error("FAIL", location=loc)
 }
 
-fail_now :: proc(t: ^T, msg := "", loc := #caller_location) {
+// fail_now will cause a test to immediately fail and abort, much in the same
+// way a failed assertion or panic call will stop a thread.
+//
+// It is for when you absolutely need a test to fail without calling any of its
+// deferred statements. It will be cleaner than a regular assert or panic,
+// as the test runner will know to expect the signal this procedure will raise.
+fail_now :: proc(t: ^T, msg := "", loc := #caller_location) -> ! {
+	t._fail_now_called = true
 	if msg != "" {
 		pkg_log.error("FAIL:", msg, location=loc)
 	} else {
 		pkg_log.error("FAIL", location=loc)
 	}
-	if t._fail_now != nil {
-		t._fail_now()
-	}
+	runtime.trap()
 }
 
 failed :: proc(t: ^T) -> bool {
@@ -94,7 +99,17 @@ logf :: proc(t: ^T, format: string, args: ..any, loc := #caller_location) {
 
 // cleanup registers a procedure and user_data, which will be called when the test, and all its subtests, complete.
 // Cleanup procedures will be called in LIFO (last added, first called) order.
-// Each procedure will use a copy of the context at the time of registering.
+//
+// Each procedure will use a copy of the context at the time of registering,
+// and if the test failed due to a timeout, failed assertion, panic, bounds-checking error,
+// memory access violation, or any other signal-based fault, this procedure will
+// run with greater privilege in the test runner's main thread.
+//
+// That means that any cleanup procedure absolutely must not fail in the same way,
+// or it will take down the entire test runner with it. This is for when you
+// need something to run no matter what, if a test failed.
+//
+// For almost every usual case, `defer` should be preferable and sufficient.
 cleanup :: proc(t: ^T, procedure: proc(rawptr), user_data: rawptr) {
 	append(&t.cleanups, Internal_Cleanup{procedure, user_data, context})
 }