Agents are bad at writing tests

Description

I was looking at the tests in #4143 and I realised once again how bad agents are at testing what they implement.
We need tests that actually test that we fix things, not that we have implemented what we have implemented.

For example:
There are at least two tests currently merged in main that provide little or no coverage of the behavior they claim to test.

1. test/test_rlhf.py::test_get_dataloader

The test parametrizes infinite=[True, False], but both branches make the exact same assertion:

if infinite:
assert not is_tensor_collection(dl)
else:
assert not is_tensor_collection(dl)

As a result, the infinite parametrization provides no coverage of the actual distinction between the two modes.

The documented behavior is that infinite=True wraps the replay buffer in create_infinite_iterator, whereas infinite=False returns the finite replay buffer directly.

The test should check that behavior instead:

• with infinite=False, exhaust the iterator and verify it raises StopIteration;
• with infinite=True, consume more than one dataset epoch and verify iteration continues.

Ideally the test should compute the number of batches in one epoch and request n_batches + 1 batches from the infinite iterator, making the check deterministic and cheap.

2. test/objectives/test_controllers.py::test_deterministic_with_zero_variance

This test calls RBFController twice with the same inputs and zero covariance and verifies that the resulting action means are equal.

However, RBFController.forward() contains no sampling. It is deterministic regardless of the input covariance. Calling any deterministic implementation twice with identical inputs will therefore satisfy this test.

Consequently, the test does not actually verify the zero-variance behavior its name suggests.

A meaningful behavioral test would verify that zero input uncertainty produces essentially zero output uncertainty, e.g. by checking the returned action covariance against zero within the tolerance required by the controller’s intentional numerical jitter.

Something along these lines:

def test_zero_input_covariance_collapses_action_covariance(self):
controller = RBFController(
input_dim=4, output_dim=1, max_action=1.0, n_basis=5
).double()
mean = torch.randn(2, 4, dtype=torch.float64)
zero_cov = torch.zeros(2, 4, 4, dtype=torch.float64)
action_mean, action_cov, _ = controller(mean, zero_cov)
assert torch.isfinite(action_mean).all()
torch.testing.assert_close(
action_cov,
torch.zeros_like(action_cov),
atol=2e-6,
rtol=0,
)

The exact tolerance should account for the intentional 1e-6 diagonal jitter in the implementation.

General principle

Regression tests should preferably assert the externally observable behavior that was broken, rather than an implementation detail or a property that would hold independently of the code path being tested.

This makes the test answer the important question: if the underlying bug is reintroduced, will this test actually fail?

We need to audit the repo for this kind of low quality test coverage, come back to the PR that added them, and replace them with more sensible ones.