Skip to content

Commit ebb9e72

Browse files
committed
Do not regenerate a conformance test the render cannot rebuild
When a conformance test exhausts its fix attempts, the loop deletes it and renders a replacement. That only works for a test the module under render owns: RenderConformanceTests builds the replacement's folder under render_context.module_name and takes its specifications from render_context.frid_context, but records the entry against current_testing_module_name. For a required module's test the files, the index entry and the specification each end up describing a different module. A test in that state also loses its recorded entry, so the next regeneration removes it with an unguarded pop and the render ends on a bare KeyError naming the functionality id - after an hour of work, with no indication of what happened. Exhausting the attempts on a required module's test is now a failed render rather than a regeneration, and the pop no longer ends a render over a state that should not arise.
1 parent b710c29 commit ebb9e72

3 files changed

Lines changed: 121 additions & 3 deletions

File tree

‎render_machine/actions/fix_conformance_test.py‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,20 @@ def execute(self, render_context: RenderContext, previous_action_payload: Any |
3131
ctx.fix_attempts += 1
3232

3333
if ctx.fix_attempts >= MAX_CONFORMANCE_TEST_FIX_ATTEMPTS:
34-
if ctx.conformance_tests_render_attempts >= MAX_CONFORMANCE_TEST_RERENDER_ATTEMPTS:
34+
# Regeneration only produces a coherent result for a test the module under
35+
# render owns. RenderConformanceTests builds the replacement's folder under
36+
# `render_context.module_name` and its specifications from
37+
# `render_context.frid_context`, but records the entry against
38+
# `current_testing_module_name` - so for a required module's test the files,
39+
# the index entry and the specification all belong to different modules.
40+
# Exhausting the fix attempts on one of those is a failed render, not a
41+
# candidate for regeneration.
42+
regeneration_would_apply_to_another_module = ctx.current_testing_module_name != render_context.module_name
43+
44+
if (
45+
ctx.conformance_tests_render_attempts >= MAX_CONFORMANCE_TEST_RERENDER_ATTEMPTS
46+
or regeneration_would_apply_to_another_module
47+
):
3548
error_msg = f"The renderer was unable to produce an implementation that passes conformance tests for functionality '{render_context.frid_context.frid}' after many attempts. Please review and rewrite the specification. (Render ID: {render_context.run_state.render_id})"
3649
render_context.last_error_message = error_msg
3750
return (

‎render_machine/render_context.py‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -396,8 +396,20 @@ def _handle_test_regeneration(self):
396396

397397
console.info(f"Recreating conformance tests for functionality {ctx.current_testing_frid}.")
398398

399-
existing_folder = ctx.get_conformance_tests_json(ctx.current_testing_module_name).pop(ctx.current_testing_frid)
400-
file_utils.delete_folder(existing_folder["folder_name"])
399+
# Popped with a default: the entry is gone if a previous regeneration removed it
400+
# and the re-render recorded the replacement elsewhere. There is then nothing to
401+
# delete, and the test is rendered fresh either way - a state that should not
402+
# arise is not worth ending a render that has run for an hour.
403+
existing_folder = ctx.get_conformance_tests_json(ctx.current_testing_module_name).pop(
404+
ctx.current_testing_frid, None
405+
)
406+
if existing_folder is None:
407+
console.warning(
408+
f"No conformance test recorded for functionality {ctx.current_testing_frid} of module "
409+
f"{ctx.current_testing_module_name}; regenerating without deleting one."
410+
)
411+
else:
412+
file_utils.delete_folder(existing_folder["folder_name"])
401413

402414
ctx.conformance_tests_render_attempts += 1
403415
ctx.fix_attempts = 0
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
"""Regenerating a conformance test after the fix attempts run out.
2+
3+
Two things went wrong on the same path. The regeneration branch was taken for any test
4+
that exhausted its fix attempts, including one belonging to a required module - which
5+
RenderConformanceTests cannot rebuild coherently, because it writes the replacement under
6+
the module being rendered while recording the entry against the module the test belongs
7+
to. And the handler removed the recorded entry with an unguarded `pop`, so a state where
8+
the entry was already gone ended the render with a bare `KeyError` naming the
9+
functionality id.
10+
"""
11+
12+
from unittest.mock import MagicMock
13+
14+
from render_machine.actions.fix_conformance_test import (
15+
MAX_CONFORMANCE_TEST_FIX_ATTEMPTS,
16+
MAX_CONFORMANCE_TEST_RERENDER_ATTEMPTS,
17+
FixConformanceTest,
18+
)
19+
from render_machine.render_context import RenderContext
20+
21+
22+
def exhausted_context(current_module="inventory_api", testing_module="inventory_api", entry=None):
23+
"""A conformance loop one attempt short of its limit, with its regeneration budget intact."""
24+
render_context = MagicMock()
25+
render_context.module_name = current_module
26+
render_context.frid_context.frid = "2"
27+
28+
ctx = render_context.conformance_tests_running_context
29+
ctx.fix_attempts = MAX_CONFORMANCE_TEST_FIX_ATTEMPTS - 1
30+
ctx.conformance_tests_render_attempts = MAX_CONFORMANCE_TEST_RERENDER_ATTEMPTS - 1
31+
ctx.current_testing_module_name = testing_module
32+
ctx.current_testing_frid = "2"
33+
ctx.get_conformance_tests_json.return_value = {} if entry is None else {"2": entry}
34+
return render_context
35+
36+
37+
class TestOnlyTheRenderedModulesTestIsRegenerated:
38+
def test_the_current_modules_test_is_regenerated(self):
39+
render_context = exhausted_context()
40+
41+
outcome, payload = FixConformanceTest().execute(render_context, None)
42+
43+
assert outcome == FixConformanceTest.REGENERATE_CONFORMANCE_TESTS_OUTCOME
44+
assert payload is None
45+
assert render_context.conformance_tests_running_context.regenerating_conformance_tests is True
46+
47+
def test_a_required_modules_test_fails_the_render_instead(self):
48+
"""The replacement would be written for the wrong module, so there is nothing to
49+
regenerate - the render has failed."""
50+
render_context = exhausted_context(current_module="inventory_api", testing_module="store_core")
51+
52+
outcome, payload = FixConformanceTest().execute(render_context, None)
53+
54+
assert outcome == FixConformanceTest.LIMIT_EXCEEDED_OUTCOME
55+
assert payload["error"]["message"]
56+
assert render_context.conformance_tests_running_context.regenerating_conformance_tests is not True
57+
58+
59+
class TestRegenerationSurvivesAMissingEntry:
60+
@staticmethod
61+
def _context_with(entry):
62+
context = RenderContext.__new__(RenderContext)
63+
ctx = MagicMock()
64+
ctx.current_testing_frid = "2"
65+
ctx.current_testing_module_name = "store_core"
66+
ctx.conformance_tests_render_attempts = 0
67+
ctx.get_conformance_tests_json.return_value = {} if entry is None else {"2": entry}
68+
context.conformance_tests_running_context = ctx
69+
return context, ctx
70+
71+
def test_a_recorded_entry_is_removed_and_its_folder_deleted(self, monkeypatch):
72+
deleted = []
73+
monkeypatch.setattr("render_machine.render_context.file_utils.delete_folder", deleted.append)
74+
context, ctx = self._context_with({"folder_name": "/tmp/tests/store_core_2"})
75+
76+
context._handle_test_regeneration()
77+
78+
assert deleted == ["/tmp/tests/store_core_2"]
79+
assert ctx.conformance_tests_render_attempts == 1
80+
assert ctx.fix_attempts == 0
81+
82+
def test_a_missing_entry_does_not_end_the_render(self, monkeypatch):
83+
"""This raised `KeyError('2')` out of the render, an hour into it, reported to the
84+
user as the bare string `'2'`."""
85+
deleted = []
86+
monkeypatch.setattr("render_machine.render_context.file_utils.delete_folder", deleted.append)
87+
context, ctx = self._context_with(None)
88+
89+
context._handle_test_regeneration()
90+
91+
assert deleted == []
92+
assert ctx.conformance_tests_render_attempts == 1
93+
assert ctx.fix_attempts == 0

0 commit comments

Comments
 (0)