Test Runner support for Dynamic Tests

[eagercoffee]

Problem Background

I'm a big fan of testing and ease of writing tests. A lot of times, several test cases share the same logic and differ only in the inputs and expected outputs. In fact, this is a current problem with the compiler tests as can be seen by the Unicode tests declaring a run_test_cases procedure (Approach A) or the Python script that generates tests for the math/big module (Approach B).

Approach A is the simplistic approach to the concept of "parameterized tests". It is easy to write and execute, with each set of inputs and outputs being considered a different test case. However, this approach does not take advantage of the test runner to execute such test cases so any guarantees, features and improvements to the test runner aren't used. This might mean threading support (e.g. if there are 1000 parameterized test cases then they all execute as a single test case in the runner, instead of the actual 1000 test cases that they are), error/panic handling, etc.

Approach B is the purest implementation of the concept of "dynamic tests". Test code is generated and then integrated into the regular test runner execution as if it existed all along. However, this approach is quite complex (in this case it involves a whole other programming language) as well as having several pitfalls: it is easy to forget to re-run the generation script, the build system now needs extra tooling for building the project, extra resources need to be maintained which might require collaborators to learn new languages or delegate the changes to someone else, etc.

Proposed Solution

The core:testing module could be extended to support submission of dynamic tests at runtime. This would allow users to rely on both the concepts of "dynamic tests" as well as "parameterized tests" (which build on top of the former). Since the dynamic tests would need to be submitted at runtime, the user has the option to:

• Continue writing parameterized tests with a simple loop through test data in a single test case (i.e. the current approach), or
• Use a data source to submit several iterations of the same test logic but with different sets of input/outputs (e.g. a pre-built array of data, a data file that's included in the project, some state that it queries from the internet, whatever is possible to do in Odin to retrieve data is a potential data source), or
• Use procedural generation of test data to generate the full data set of test scenarios (if finite and small), generate several iterations of boundary conditions or even generate random samples of the possible input universe in order to do some property-based testing

The main point here is that the developer has full control on how much or how little to rely on the runtime submission of dynamic tests, while opening up a whole breadth of potential testing power by using the technology that they are already familiar with: Odin. And for users that do not use this feature entirely there would be no impact to their regular testing workflow.

Additional Notes

This is something I have wanted for a long time so I played around with implementing it on my local machine, and to my surprise it was much simpler than I originally expected. It obviously isn't the quality required for integration with the code base (e.g. I haven't cleaned up the changes to remove code duplication, naming improvements, etc.) and I don't have the means to validate that it works across all targets, but it works as a proof of concept. I'm including the git patch here as a code reference to what I am proposing.

Dynamic Test Support Patch - PoC

diff --git a/core/testing/reporting.odin b/core/testing/reporting.odin
index 7c7eb7b..3b94454 100644
--- a/core/testing/reporting.odin
+++ b/core/testing/reporting.odin
@@ -57,7 +57,7 @@ Report :: struct {
 	progress_width: int,
 
 	all_tests: []Internal_Test,
-	all_test_states: []Test_State,
+	all_test_states: [dynamic]Test_State,
 }
 
 // Organize all tests by package and sort out test state data.
@@ -67,7 +67,7 @@ make_report :: proc(internal_tests: []Internal_Test) -> (report: Report, error:
 	packages: [dynamic]Package_Run
 
 	report.all_tests = internal_tests
-	report.all_test_states = make([]Test_State, len(internal_tests)) or_return
+	resize(&report.all_test_states, len(internal_tests)) or_return
 
 	// First, figure out what belongs where.
 	#no_bounds_check cur_pkg := internal_tests[0].pkg
diff --git a/core/testing/runner.odin b/core/testing/runner.odin
index b53bd87..842a34e 100644
--- a/core/testing/runner.odin
+++ b/core/testing/runner.odin
@@ -281,6 +281,7 @@ runner :: proc(internal_tests: []Internal_Test) -> bool {
 	total_failure_count := 0
 	total_success_count := 0
 	total_done_count    := 0
+	total_static_tests  := len(internal_tests)
 	total_test_count    := len(internal_tests)
 
 
@@ -319,8 +320,6 @@ runner :: proc(internal_tests: []Internal_Test) -> bool {
 		thread_count := max(1, TEST_THREADS)
 	}
 
-	thread_count = min(thread_count, total_test_count)
-
 	// -- Allocate.
 
 	pool_stack: mem.Rollback_Stack
@@ -382,6 +381,8 @@ runner :: proc(internal_tests: []Internal_Test) -> bool {
 	}
 	defer delete(thread_count_status_string)
 
+	dynamic_tests: [dynamic]Dynamic_Test
+	defer delete(dynamic_tests)
 
 	task_data_slots: []Task_Data = ---
 	task_data_slots, alloc_error = make([]Task_Data, thread_count)
@@ -437,6 +438,9 @@ runner :: proc(internal_tests: []Internal_Test) -> bool {
 	fmt.assertf(alloc_error == nil, "Error allocating memory for sorted failed test reasons: %v", alloc_error)
 	defer delete(sorted_failed_test_reasons)
 
+	pending_threads: [dynamic]thread.Task
+	defer delete(pending_threads)
+
 	when USE_CLIPBOARD {
 		clipboard_buffer: bytes.Buffer
 		bytes.buffer_init_allocator(&clipboard_buffer, 0, CLIPBOARD_BUFFER_SIZE)
@@ -472,25 +476,26 @@ runner :: proc(internal_tests: []Internal_Test) -> bool {
 	run_index: int
 
 	setup_tasks: for &data, task_index in task_data_slots {
+		data.t.seed = shared_random_seed
+		#no_bounds_check data.t.channel = chan.as_send(task_channels[task_index].channel)
+		data.t._log_allocator = shared_log_allocator
+		data.t.dyn_test_submissions.allocator = shared_log_allocator
+		data.allocator_index = task_index
+
+		#no_bounds_check when TRACKING_MEMORY {
+			task_allocator := mem.tracking_allocator(&task_memory_trackers[task_index])
+			when FAIL_ON_BAD_MEMORY {
+				data.tracking_allocator = &task_memory_trackers[task_index]
+			}
+		} else {
+			task_allocator := mem.rollback_stack_allocator(&task_allocators[task_index])
+		}
+
 		setup_next_test: for run_index < total_test_count {
 			#no_bounds_check it := internal_tests[run_index]
 			defer run_index += 1
 
 			data.it = it
-			data.t.seed = shared_random_seed
-			#no_bounds_check data.t.channel = chan.as_send(task_channels[task_index].channel)
-			data.t._log_allocator = shared_log_allocator
-			data.allocator_index = task_index
-
-			#no_bounds_check when TRACKING_MEMORY {
-				task_allocator := mem.tracking_allocator(&task_memory_trackers[task_index])
-				when FAIL_ON_BAD_MEMORY {
-					data.tracking_allocator = &task_memory_trackers[task_index]
-				}
-			} else {
-				task_allocator := mem.rollback_stack_allocator(&task_allocators[task_index])
-			}
-
 			thread.pool_add_task(&pool, task_allocator, run_test_task, &data, run_index)
 
 			continue setup_tasks
@@ -594,19 +599,80 @@ runner :: proc(internal_tests: []Internal_Test) -> bool {
 				mem.tracking_allocator_reset(tracker)
 			}
 
+			if len(data.t.dyn_test_submissions) > 0 {
+				for submission in data.t.dyn_test_submissions {
+					dyn_test := Dynamic_Test { internal = data.it, data = submission.data }
+					dyn_test.p = submission.procedure
+					_, alloc_error = append(&dynamic_tests, dyn_test)
+					fmt.assertf(alloc_error == nil, "Error allocating memory for submitted dynamic tests: %v", alloc_error)
+				}
+
+				total_test_count += len(data.t.dyn_test_submissions)
+				clear(&data.t.dyn_test_submissions)
+
+				alloc_error = resize(&report.all_test_states, total_test_count)
+				fmt.assertf(alloc_error == nil, "Error allocating memory for reporting of dynamic tests: %v", alloc_error)
+			}
+
 			free_all(task.allocator)
 
 			if run_index < total_test_count {
-				#no_bounds_check it := internal_tests[run_index]
+				it: Internal_Test
+				if run_index < total_static_tests {
+					#no_bounds_check it	= internal_tests[run_index]
+				} else {
+					#no_bounds_check it	= dynamic_tests[run_index - total_static_tests]
+				}
 				defer run_index += 1
 
 				data.it = it
 				data.t.seed = shared_random_seed
 				data.t.error_count = 0
 				data.t._fail_now_called = false
+				if run_index >= total_static_tests {
+					data.t.dyn_data = dynamic_tests[run_index - total_static_tests].data
+
+					context.user_ptr = &data.t
+					cleanup(&data.t, proc(ptr: rawptr) {
+						t := cast(^T) context.user_ptr
+						free(ptr, t._log_allocator)
+					}, data.t.dyn_data)
+				}
 
 				thread.pool_add_task(&pool, task.allocator, run_test_task, data, run_index)
+			} else {
+				_, alloc_error = append(&pending_threads, task)
+				fmt.assertf(alloc_error == nil, "Error allocating memory for storing pending threads: %v", alloc_error)
+			}
+		}
+
+		dispatch_pending_threads: for run_index < total_test_count && len(pending_threads) > 0 {
+			task := pop(&pending_threads)
+			data := cast(^Task_Data)(task.data)
+
+			it: Internal_Test
+			if run_index < total_static_tests {
+				#no_bounds_check it	= internal_tests[run_index]
+			} else {
+				#no_bounds_check it	= dynamic_tests[run_index - total_static_tests]
+			}
+			defer run_index += 1
+
+			data.it = it
+			data.t.seed = shared_random_seed
+			data.t.error_count = 0
+			data.t._fail_now_called = false
+			if run_index >= total_static_tests {
+				data.t.dyn_data = dynamic_tests[run_index - total_static_tests].data
+
+				context.user_ptr = &data.t
+				cleanup(&data.t, proc(ptr: rawptr) {
+					t := cast(^T) context.user_ptr
+					free(ptr, t._log_allocator)
+				}, data.t.dyn_data)
 			}
+
+			thread.pool_add_task(&pool, task.allocator, run_test_task, data, run_index)
 		}
 
 		handle_events: for &task_channel in task_channels {
@@ -618,7 +684,12 @@ runner :: proc(internal_tests: []Internal_Test) -> bool {
 				case Event_State_Change:
 					#no_bounds_check report.all_test_states[task_channel.test_index] = event.new_state
 
-					#no_bounds_check it := internal_tests[task_channel.test_index]
+					it: Internal_Test
+					if task_channel.test_index < total_static_tests {
+						#no_bounds_check it	= internal_tests[task_channel.test_index]
+					} else {
+						#no_bounds_check it	= dynamic_tests[task_channel.test_index - total_static_tests]
+					}
 					#no_bounds_check pkg := report.packages_by_name[it.pkg]
 
 					#partial switch event.new_state {
@@ -691,7 +762,12 @@ runner :: proc(internal_tests: []Internal_Test) -> bool {
 			}
 
 			#no_bounds_check report.all_test_states[timeout.test_index] = .Failed
-			#no_bounds_check it := internal_tests[timeout.test_index]
+			it: Internal_Test
+			if timeout.test_index < total_static_tests {
+				#no_bounds_check it	= internal_tests[timeout.test_index]
+			} else {
+				#no_bounds_check it	= dynamic_tests[timeout.test_index - total_static_tests]
+			}
 			#no_bounds_check pkg := report.packages_by_name[it.pkg]
 			pkg.frame_ready = false
 
diff --git a/core/testing/testing.odin b/core/testing/testing.odin
index 1357a46..bac03dc 100644
--- a/core/testing/testing.odin
+++ b/core/testing/testing.odin
@@ -33,6 +33,10 @@ Internal_Test :: struct {
 	p:    Test_Signature,
 }
 
+Dynamic_Test :: struct {
+	using internal: Internal_Test,
+	data: rawptr,
+}
 
 Internal_Cleanup :: struct {
 	procedure: proc(rawptr),
@@ -40,6 +44,11 @@ Internal_Cleanup :: struct {
 	ctx: runtime.Context,
 }
 
+Test_Submission :: struct {
+	procedure: Test_Signature,
+	data: rawptr,
+}
+
 T :: struct {
 	error_count: int,
 
@@ -63,6 +72,9 @@ T :: struct {
 	_log_allocator: runtime.Allocator,
 
 	_fail_now_called: bool,
+
+	dyn_test_submissions: [dynamic]Test_Submission,
+	dyn_data: rawptr,
 }
 
 
@@ -107,6 +119,25 @@ cleanup :: proc(t: ^T, procedure: proc(rawptr), user_data: rawptr) {
 	append(&t.cleanups, Internal_Cleanup{procedure, user_data, context})
 }
 
+submit_dyn_test :: proc(t: ^T, data: rawptr, size: int, procedure: Test_Signature) {
+	// Use the log allocator to outlive the test
+	ptr, err := mem.alloc(size, allocator = t._log_allocator)
+	if expectf(t, err == .None, "could not submit dynamic test: %v", err) {
+		append(&t.dyn_test_submissions, Test_Submission {
+			procedure = procedure,
+			data = mem.copy(ptr, data, size),
+		})
+	}
+}
+
+get_dyn_data :: proc(t: ^T, $V: typeid) -> (ret: V) {
+	if expect(t, t.dyn_data != nil, "there is no dynamic data associated with this test") {
+		ret = (cast(^V) t.dyn_data)^
+		return
+	}
+	return
+}
+
 expect :: proc(t: ^T, ok: bool, msg := "", expr := #caller_expression(ok), loc := #caller_location) -> bool {
 	if !ok {
 		if msg == "" {

[leecommamichael]

Am I understanding correctly that this is essentially an API for threading test-cases? I'm reading it as if you're able to wrap what used to be the body of a loop with something and now the inner test case of that loop is treated as many test cases which run concurrently.

[eagercoffee]

That is essentially the core of it. The API allows you to register several combinations of a procedure + an arbitrary data pointer and have each combination be a test case (for example). The runner then adds those as pending tests and runs them as part of the normal test framework, making sure they handle panics correctly, potentially execute across multiple threads, have automatic memory tracking, etc. The user then has free reign about whether they use this feature or not, and if they do then that unlocks the potential for a lot of different data sources or dynamic test cases to be created.

The big advantage of this is really the fact that dynamic tests execute as regular tests with the full capabilities and guarantees of the test runner. This can currently only be achieved by generating source code for the multiple tests (either manually or automatically, but both involve a lot of manual labour and/or extra build steps).

[Feoramund]

In fact, this is a current problem with the compiler tests as can be seen by the Unicode tests declaring a run_test_cases procedure (Approach A) [...]

This is not a problem. Turning one test which runs well into 6,000 tests which compete for all of the cores of the machine while shuffling the same data between caches when it could instead be focused in one spot is not a good idea.

However, this approach does not take advantage of the test runner to execute such test cases so any guarantees, features and improvements to the test runner aren't used.

This is false. The only difference between your proposal and now is that the test vectors would be given their own thread.

This might mean threading support (e.g. if there are 1000 parameterized test cases then they all execute as a single test case in the runner, instead of the actual 1000 test cases that they are), error/panic handling, etc.

Correct. The situation as it is now is optimal for the case you have outlined as a problem.

If you need automated subdivisions of tests (i.e. @test-level tests), I recommend writing a code generator for them on the basis of your test dataset.

This is something I have wanted for a long time so I played around with implementing it on my local machine, and to my surprise it was much simpler than I originally expected.

If this is something you truly want, there is nothing stopping you from maintaining your own test runner, but as it stands, I am not welcoming the added complexity for what is yet to be illustrated to me as a problem.

[eagercoffee]

This is not a problem. Turning one test which runs well into 6,000 tests which compete for all of the cores of the machine while shuffling the same data between caches when it could instead be focused in one spot is not a good idea.

Phrasing it as a problem might have been wrong on my end, I apologise for that, but it is a valid example of the general approach of encoding several parameterized test cases as a loop because of lack of proper support. I'm not saying that approach is necessarily bad, hence why I mention that the power of the decision is still up to the user, but in some other test cases it might be preferrable. One example is my own project where I decided to implement the PoC for. It used this same approach to run 8800 test cases, but turning it into the dynamic test approach cut down the test execution time in half (even with the extra setup required for the dynamic test execution).

This is false. The only difference between your proposal and now is that the test vectors would be given their own thread.

I might be wrong about this, after all my investigation of the runner implementation was mostly geared towards patching in some support for dynamic tests, but all guarantees provided by the test runner are on a general test case basis, correct? In most cases this won't have an impact, but in some it will. One such example is threading, as you mentioned, but I believe another situation is panic handling. If I use a simple loop on a bunch of different test data and somewhere in the middle a panic gets triggered then the remainder of the loop (and therefore test data) won't get processed, correct? It is true that the test runner still provides the overarching panic handling, but in that case it doesn't provide it on an individual test data level because in its view it is still just one single test execution. But if I'm wrong on this analysis then please let me know, as I mentioned my knowledge on the runner implementation is shallow, especially when compared to you who actually implemented it, and I've already learned a lot just from the analysis I did do.

Correct. The situation as it is now is optimal for the case you have outlined as a problem.

If you need automated subdivisions of tests (i.e. @test-level tests), I recommend writing a code generator for them on the basis of your test dataset.

The issue I personally have with this is that then you are complicating the build/development process of the project. You start requiring an extra external tool/step just to ensure you then either adapt or generate the tests in an optimal way for the test runner, when in my perspective that should be part of testing tooling to begin with. There is already control on the current version in the form of configuring how many threads the tests can run in, but that data segregation/test subdivision, or more generally dynamic test support, is not currently present. It might not make sense to add it though, but that's why I raised this proposal as well, so it could be discussed and a conclusion be reached. For my personal purposes I already have the PoC that currently covers my needs, but if I could push the Odin tooling just the tiniest smidge forward then I'd be more than happy.

[Feoramund]

One such example is threading, as you mentioned, but I believe another situation is panic handling. If I use a simple loop on a bunch of different test data and somewhere in the middle a panic gets triggered then the remainder of the loop (and therefore test data) won't get processed, correct?

That's correct. If a test vector panics, the test itself will go down. I consider this fine because I'd rather investigate the test as a whole and I am not afforded any benefit by having the other tests run at the expense of the complexity introduced by managing dynamic test state.

If you have a panic, you have a serious problem. It shouldn't be treated as if it's a normal thing that is mitigated by having more threaded tasks to execute.

The issue I personally have with this is that then you are complicating the build/development process of the project.

The complexity of this solution has to go somewhere: either in a user's project or in the core, and as it stands, the whole of the Odin core library has not seen a need for something like this. The Odin test suite composed of 490 tests which grants us ~76% total coverage of core executes in 2.6 seconds on my machine. This is with no optimizations.

If you have a situation where you have long-running test vectors, you can do what we've done in the test suite, which is to partition them off in a separate build unit that is then built with -o:speed apart from your normal tests. Alternatively (and not mutually exclusively), use a code generator.

I prefer there to be some friction for this, as it gives the programmer pause to consider whether or not it's worth it to do. If we have dynamic tests easily available, then it could cause taxonomical confusion as to when is it a good idea to have a @test and when is it a good idea to have a runtime-added test and lead to the writing of atomized suboptimal tests. I don't want to encourage this.

The simpler (less complicated) that Odin's ecosystem is, the better.

[eagercoffee]

I have a somewhat different opinion in some of the points you raised (mostly around the separate build unit that is run with different build flags, meaning the output you're testing has a potential for differences), but I can generally understand your point of view. In the end if the main goal is to keep the Odin ecosystem as simple as possible then I can understand this proposal being denied. Thanks for the discussion 😸