test: scripted end-to-end tests

[benthevining]

• Load/save state from config file
• Check that engine info prints correct version
• Try setting invalid option values, invalid option names

In general, to fix this class of problems, move any side-effecting function calls out of assert and into normal control flow, and restrict assert to pure boolean checks (or remove the assert if the check is nonessential). This ensures behavior is the same regardless of whether assertions are enabled.

For this specific case in tests/e2e/testing.py, we should separate the side-effect (self.close()) from the assertion. The destructor should always perform cleanup, not only when assertions are active. The safest change that preserves existing semantics is:

1. Call self.close() unconditionally in __del__, storing the return code.
2. Optionally assert on that return code, but only after the call.
3. Alternatively, just drop the assertion if it's not needed for test correctness.

Because __del__ is best-effort cleanup and must not raise exceptions during interpreter shutdown, the most robust approach is to call self.close() and ignore or log the result instead of asserting it. That also avoids raising AssertionError in a destructor. However, to stay as close as possible to current behavior while removing the side-effect from the assert, we can compute the return code in a variable and then assert on that variable. To avoid raising from __del__ at all, we should also wrap this in a try/except.

Concretely, around line 220–222:

    def __del__(self):
        self.quit()
        assert self.close() == 0

We can change it to:

    def __del__(self):
        try:
            self.quit()
            rc = self.close()
            assert rc == 0
        except Exception:
            # Avoid exceptions escaping from __del__
            pass

This preserves the side effects regardless of optimization, and the assert now only operates on a stored value.

No new imports or helper methods are required.

---

In general, to fix “commented-out code” you either (a) restore it as real code if it is still needed and correct, or (b) delete it if it is obsolete or redundant. Leaving commented-out code in place is discouraged because it is not executed or tested and adds noise.

Here, the best fix that does not change existing behavior is to remove the commented-out test methods test_position_3 and test_position_4 entirely from TestEnPassantSanitization. The currently active tests (test_position_1, test_position_2, test_position_5, test_position_6) will remain unchanged, and the test runner behavior is preserved: no active tests are added or removed, and no imports or helper methods need to be introduced. We simply delete lines 258–272 (the commented-out function definitions and their bodies).

All required functionality (argparse, test framework, engine wrapper) already exists; no new methods, imports, or definitions are necessary.

In general, to fix commented-out code, either (1) restore it to active code in a correct, maintained form, or (2) remove it entirely if it is obsolete or not intended to run. For tests, leaving them commented out is especially problematic because it hides missing coverage and can confuse maintainers.

In this case, the commented-out methods test_position_3, test_position_4, and test_position_8 follow an older instance-style pattern (self.engine) that does not match the current staticmethod pattern used by the active tests in TestEnPassantSanitization. Reinstating them would require nontrivial refactoring and could change test behavior. Since we must avoid changing existing functionality, the safest, minimal-impact fix is to delete these commented-out blocks altogether. This maintains current behavior while removing confusing, dead code.

Concretely, in tests/e2e/e2e.py, within class TestEnPassantSanitization, remove the fully commented blocks for test_position_3, test_position_4, and test_position_8 (lines 258–265, 266–272, and the later commented test_position_8 block). No new imports, methods, or definitions are required; we are only deleting comments.

To fix the problem, remove the commented-out test methods that look like inactive code instead of documentation. This eliminates confusion, ensures the file only contains active tests or clearly non-code comments, and complies with the guideline against commented-out code.

Concretely, in tests/e2e/e2e.py, delete the fully commented-out test_position_8 and test_position_9 blocks (lines 298–305 and 306–313 in the snippet). No imports, helpers, or functional behavior elsewhere need to change, because these tests are already disabled and not referenced. We simply remove those comment lines; all active tests (test_position_5, 6, 7, 10, 11) and the rest of the file remain untouched.

In general, to fix commented-out code you either (a) delete it if it is no longer needed, relying on version control to recover it later, or (b) reinstate it as active, properly integrated code (or convert it into clear non-code documentation).

Here, the commented-out blocks test_position_8 and test_position_9 sit between active tests and look like normal tests. Without any indication that they are deliberately disabled (e.g., via a test-skip mechanism), the cleanest fix that doesn’t change existing functionality is to remove these commented-out definitions entirely, since they are not currently part of the test suite’s behavior. That eliminates the “commented-out code” smell while preserving the active behavior of the tests.

Concretely:

• In tests/e2e/e2e.py, remove lines 298–305 (the commented-out test_position_8) and lines 306–312 (the commented-out test_position_9).
• No new imports or helpers are required.

[github-actions]

Test Results (pull_request)

   13 files     13 suites   2h 35m 11s ⏱️
  222 tests   210 ✅ 0 💤 12 ❌
2 535 runs  2 503 ✅ 0 💤 32 ❌

For more details on these failures, see this check.

Results for commit 987c106.