Replace assertion-free mock tests with tests that actually execute (2,513 mock occurrences) #117

Part of #108 · Phase 2 · Depends on the production-mock-removal issue

Problem

60 of 240 test files use mocks; 2,513 total occurrences of unittest.mock|MagicMock|AsyncMock|@patch|patch(|monkeypatch|Mock(. Three of them are inside tests/real_world/, defeating the purpose of that directory entirely.

The project's own rules state: "Do not use mock services for anything ever" and "All tests need to use 'real' function calls."

Representative offenders — tests that mock exactly the thing they claim to test

1. tests/test_executor_schedulers.py:73-131 test_submit_slurm_job_basic — patches tempfile.NamedTemporaryFile, pickle.dump, os.unlink, setup_remote_environment, create_job_script, and stubs connection_manager.execute_remote_command to return "Submitted batch job 12345". Then asserts job_id == "12345". It tests a regex against a string the test itself supplied. No SLURM is involved. Same pattern at :139, :212, :262, :321, :409.
2. tests/test_executor.py:79 test_execute_command — @patch("paramiko.SSHClient"), sets mock_stdout.read.return_value = b"command output", asserts stdout == "command output". Asserts a mock returns what it was told to return.
3. tests/test_ssh_automation.py:123 test_validate_ssh_key_success — mocks paramiko.SSHClient, asserts validate_ssh_key(...) is True. No key is ever validated.
4. tests/test_filesystem.py:205 test_remote_ls — mocks SSH, feeds b"file1.txt\nfile2.py\nsubdir/\n", asserts ls() returns those three names. Tests str.split and rstrip.
5. tests/test_cloud_providers_aws.py:42-44 test_authenticate_success — patches BOTO3_AVAILABLE = True and boto3. Passes on a machine with no boto3 installed at all.
6. tests/test_kubernetes_integration.py:31 — fixture patches kubernetes.client wholesale; create_namespaced_job returns a Mock whose .metadata.name is "test-job-123". No cluster, no API.
7. tests/test_executor_connections.py:72,103,144,190,215,234,253,279,295 — every connection test patches paramiko.SSHClient. The connection manager is never tested against a socket.

The standard to replace them with

For each subsystem, in preference order:

1. Real execution against a real target. SSH/SLURM/PBS/SGE -> a container or the HF Jobs substrate. Filesystem -> a real temp dir and a real SFTP session.
2. Real local server. Spin an actual sshd in a container; connect a real socket. This is what test_executor_connections.py should do.
3. Recorded real interactions (e.g. vcrpy for pricing APIs) — recorded once from a genuine call, per the project's "verify with real calls, then record" rule for paid APIs.
4. Delete. A test that only asserts a mock's configured return value has negative value: it costs maintenance and creates false confidence. Deleting it raises the quality of the suite.

Acceptance criteria

• grep -rn "unittest.mock" tests/real_world/ returns zero — that directory is real-only by definition
• Every test named in the list above is either rewritten against a real target or deleted with a stated reason
• Mock occurrences in tests/ reduced by at least an order of magnitude from 2,513
• Each supported backend has at least one test that submits a real job and asserts on a real returned value
• No test asserts equality against a string the same test supplied to a stub

Note

Expect the pass count to drop as this lands, and expect real bugs to surface. That is the point — the cloud-path KeyError in Phase 3 survived precisely because tests mocked past it.