smite-ir: Add unit tests for OperationParamMutator

[Chand-ra]

This PR is a follow-up to #44, adding targeted, deterministic unit tests for the OperationParamMutator's helper functions.

Current tests for this mutator rely primarily on top-level integration loops. Directly testing the helper functions with seeded RNGs guarantees that every individual mutation branch is structurally sound and prevents future regressions (like off-by-one errors).

[ekzyis]

GitHub is having issues (again). Couldn't create review comments for individual lines, sorry.

L340:

-        assert_ne!(shuffled, sorted, "Shuffle must shuffle elements");
+        assert_ne!(shuffled, sorted, "shuffle_subrange must shuffle elements");

or

-        assert_ne!(shuffled, sorted, "Shuffle must shuffle elements");
+        assert_ne!(shuffled, sorted, "must shuffle elements");

---

L344: same suggestion

---

L424: I think "field that has NO other fields" is confusing

---

L425:

u32 is slightly inaccurate, block heights are their own type:

-        // MinimumDepth is the only field with a `u32` output type.
+        // MinimumDepth is the only field with a `BlockHeight` output type.

[morehouse]

Most of these tests are already covered in the public API tests. I think we should drop all the tests except the shuffle_subrange ones that actually provide some benefit.

[morehouse]

I think we also should test that shuffle generally does something.

Suggested change

	let mut shuffled = sorted.clone();
	for _ in 0..100 {
	shuffle_subrange(&mut shuffled, &mut rng);
	shuffled.sort_unstable();
	assert_eq!(
	shuffled, sorted,
	"shuffle_subrange must preserve all original elements"
	);
	}
	let mut shuffled = sorted.clone();
	let mut diff_count = 0;
	for _ in 0..100 {
	shuffle_subrange(&mut shuffled, &mut rng);
	if shuffled != sorted {
	diff_count += 1;
	}
	shuffled.sort_unstable();
	assert_eq!(
	shuffled, sorted,
	"shuffle_subrange must preserve all original elements"
	);
	}
	assert(diff_count >= 70, "shuffle_subrange frequently does nothing");

[Chand-ra]

Doesn't look quite right to me.

Since our RNG is deterministic, diff_count will evaluate to the exact same number every time for a given seed, at which point we're more so testing the statistical distribution of the rand crate rather than the code's logic.

I believe it's better to test that shuffle_subrange mutates the input at least once. This way we can be sure the function isn't broken and just returning the identical array all the time.

[morehouse]

The idea was to test that there isn't some unexpected bias towards doing nothing (either from our mutator implementation or the RNG), while also reducing the test flakiness in case later code changes cause slightly different mutations to occur.

But either way this is a strict improvement in testing coverage.

[Chand-ra]

Rebased on top of the latest master to resolve a merge conflict.

[morehouse]

The idea was to test that there isn't some unexpected bias towards doing nothing (either from our mutator implementation or the RNG), while also reducing the test flakiness in case later code changes cause slightly different mutations to occur.

But either way this is a strict improvement in testing coverage.