Skip to content

Commit 4ccbe9b

Browse files
committed
Hand the conformance tests fix to the unit tests fixer
When the conformance tests fixer changes implementation code, the unit tests are re-run and, if they fail, fixed next. The unit tests fixer had no idea the implementation was just deliberately changed, so it repaired the implementation back to what the stale unit tests asserted. The conformance fixer then re-applied the same change, and the render looped. The conformance tests running context now records every implementation change the conformance fixer made (hypothesis, approach and per-file diff). FixUnitTests sends that list as conformance_tests_fixes with the /fix_unittests_issue request, so the fixer preserves the change and adjusts the unit tests instead. The field is only sent when unit tests are processed inside the conformance phase.
1 parent 2a58895 commit 4ccbe9b

5 files changed

Lines changed: 276 additions & 0 deletions

File tree

‎codeplain_REST_api.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,7 @@ def fix_unittests_issue(
237237
required_modules,
238238
unittests_issue,
239239
run_state: RunState,
240+
conformance_tests_fixes: list[dict] | None = None,
240241
):
241242
endpoint_url = f"{self.api_url}/fix_unittests_issue"
242243
headers = {"X-API-Key": self.api_key, "Content-Type": "application/json"}
@@ -251,6 +252,10 @@ def fix_unittests_issue(
251252
"unittests_issue": unittests_issue,
252253
"unittest_batch_id": run_state.unittest_batch_id,
253254
}
255+
# Implementation code changes made by the conformance tests fixer right before this unit tests run;
256+
# only sent when unit tests are processed inside the conformance tests phase.
257+
if conformance_tests_fixes is not None:
258+
payload["conformance_tests_fixes"] = conformance_tests_fixes
254259

255260
return self.post_request(endpoint_url, headers, payload, run_state)
256261

‎render_machine/actions/fix_conformance_test.py‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,17 @@ def execute(self, render_context: RenderContext, previous_action_payload: Any |
189189
ctx.test_that_triggered_code_change = (ctx.current_testing_module_name, ctx.current_testing_frid)
190190
ctx.execution_phase = TestExecutionPhase.RETRYING_AFTER_CODE_CHANGE
191191

192+
# Remember the change so that the unit tests fixer (which runs next) preserves it instead of
193+
# reverting it - see FixUnitTests.
194+
summary = fix_attempt_summary if isinstance(fix_attempt_summary, dict) else {}
195+
ctx.implementation_code_fixes.append(
196+
{
197+
"hypothesis": summary.get("hypothesis"),
198+
"approach": summary.get("approach"),
199+
"code_diff": code_diff_files_content,
200+
}
201+
)
202+
192203
journal.record_fix(
193204
ctx.current_testing_module_name,
194205
ctx.current_testing_frid,

‎render_machine/actions/fix_unit_tests.py‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,13 @@ def execute(self, render_context: RenderContext, previous_action_payload: Any |
3232

3333
render_utils.print_inputs(render_context, existing_files_content, "Files sent as input to unit tests fixing:")
3434

35+
conformance_tests_fixes = self._get_conformance_tests_fixes(render_context)
36+
if conformance_tests_fixes:
37+
console.info(
38+
f"Unit tests are fixed while preserving {len(conformance_tests_fixes)} implementation code change(s) "
39+
"made to fix the conformance tests."
40+
)
41+
3542
response_files = render_context.codeplain_api.fix_unittests_issue(
3643
render_context.frid_context.frid,
3744
render_context.plain_source_tree,
@@ -41,6 +48,7 @@ def execute(self, render_context: RenderContext, previous_action_payload: Any |
4148
render_context.get_required_modules_functionalities(),
4249
previous_unittests_issue,
4350
run_state=render_context.run_state,
51+
conformance_tests_fixes=conformance_tests_fixes,
4452
)
4553

4654
_, changed_files = file_utils.update_build_folder_with_rendered_files(
@@ -52,3 +60,20 @@ def execute(self, render_context: RenderContext, previous_action_payload: Any |
5260
console.print_files("Files fixed:", render_context.build_folder, response_files, style=console.OUTPUT_STYLE)
5361

5462
return self.SUCCESSFUL_OUTCOME, None
63+
64+
@staticmethod
65+
def _get_conformance_tests_fixes(render_context: RenderContext) -> list[dict] | None:
66+
"""Implementation code changes the conformance tests fixer made before these unit tests were run.
67+
68+
Only present when unit tests are processed inside the conformance tests phase - the implementation
69+
and refactoring unit test passes have no conformance tests running context.
70+
"""
71+
conformance_tests_running_context = getattr(render_context, "conformance_tests_running_context", None)
72+
if conformance_tests_running_context is None:
73+
return None
74+
75+
implementation_code_fixes = getattr(conformance_tests_running_context, "implementation_code_fixes", None)
76+
if not implementation_code_fixes:
77+
return None
78+
79+
return list(implementation_code_fixes)

‎render_machine/render_types.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,11 @@ def __init__(
9191

9292
self.current_testing_frid_high_level_implementation_plan: Optional[str] = None
9393

94+
# Implementation code changes made by the conformance tests fixer during this conformance phase, in
95+
# order. Each entry is {"hypothesis": str | None, "approach": str | None, "code_diff": {file: diff}}.
96+
# Handed to the unit tests fixer so it adjusts the unit tests instead of reverting these changes.
97+
self.implementation_code_fixes: list[dict] = []
98+
9499
def get_conformance_tests_json(self, module_name: str) -> dict:
95100
return self._conformance_tests_json[module_name]
96101

Lines changed: 230 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,230 @@
1+
"""Tests for handing the conformance tests fixer's implementation changes to the unit tests fixer.
2+
3+
When the conformance tests fixer changes implementation code, the unit tests are re-run and fixed next.
4+
The unit tests fixer must know about that change so it adjusts the unit tests instead of reverting it.
5+
"""
6+
7+
import os
8+
import tempfile
9+
from types import SimpleNamespace
10+
11+
import pytest
12+
13+
import plain_spec
14+
from memory_management import MemoryManager
15+
from render_machine.actions import fix_unit_tests as fix_unit_tests_module
16+
from render_machine.actions.fix_conformance_test import FixConformanceTest
17+
from render_machine.actions.fix_unit_tests import FixUnitTests
18+
from render_machine.implementation_code_helpers import ImplementationCodeHelpers
19+
from render_machine.render_types import ConformanceTestsRunningContext, UnitTestsRunningContext
20+
21+
22+
class FakeCodeplainAPI:
23+
def __init__(self, conformance_fix_response=None, unittests_fix_response=None):
24+
self.conformance_fix_response = conformance_fix_response
25+
self.unittests_fix_response = unittests_fix_response or {}
26+
self.unittests_fix_calls = []
27+
28+
def fix_conformance_tests_issue(self, *args, **kwargs):
29+
return self.conformance_fix_response
30+
31+
def fix_unittests_issue(self, *args, **kwargs):
32+
self.unittests_fix_calls.append(kwargs)
33+
return self.unittests_fix_response
34+
35+
36+
class FakeConformanceTests:
37+
def fetch_existing_conformance_test_files(self, *args):
38+
return {}, {}
39+
40+
def store_conformance_tests_files(self, *args):
41+
pass
42+
43+
44+
@pytest.fixture
45+
def build_folder():
46+
with tempfile.TemporaryDirectory() as folder:
47+
with open(os.path.join(folder, "app.py"), "w", encoding="utf-8") as source_file:
48+
source_file.write("def add(a, b):\n return a + b\n")
49+
yield folder
50+
51+
52+
@pytest.fixture
53+
def memory_folder():
54+
with tempfile.TemporaryDirectory() as folder:
55+
yield folder
56+
57+
58+
@pytest.fixture(autouse=True)
59+
def isolate_from_git_and_console(monkeypatch):
60+
monkeypatch.setattr(ImplementationCodeHelpers, "get_code_diff", staticmethod(lambda *args: {}))
61+
monkeypatch.setattr(plain_spec, "collect_linked_resources", lambda *args: None)
62+
monkeypatch.setattr(fix_unit_tests_module.render_utils, "print_inputs", lambda *args: None)
63+
64+
65+
def make_conformance_context():
66+
return ConformanceTestsRunningContext(
67+
current_testing_module_name="mod",
68+
current_testing_frid="1",
69+
fix_attempts=0,
70+
conformance_tests_json={"1": {"folder_name": "conformance_1"}},
71+
conformance_tests_render_attempts=0,
72+
current_testing_frid_specifications={},
73+
should_prepare_testing_environment=False,
74+
frid_being_implemented="1",
75+
)
76+
77+
78+
def make_render_context(api, build_folder, memory_folder, conformance_tests_running_context):
79+
return SimpleNamespace(
80+
codeplain_api=api,
81+
build_folder=build_folder,
82+
memory_manager=MemoryManager(api, memory_folder),
83+
conformance_tests=FakeConformanceTests(),
84+
conformance_tests_running_context=conformance_tests_running_context,
85+
unit_tests_running_context=UnitTestsRunningContext(fix_attempts=1),
86+
plain_source_tree={},
87+
module_name="mod",
88+
required_modules=None,
89+
frid_context=SimpleNamespace(frid="1", linked_resources={}),
90+
get_required_modules_functionalities=lambda: {},
91+
run_state=SimpleNamespace(render_id="test-render-id", unittest_batch_id=1),
92+
)
93+
94+
95+
def run_conformance_fix(render_context):
96+
return FixConformanceTest().execute(render_context, {"previous_conformance_tests_issue": "tests failed"})
97+
98+
99+
def test_conformance_fix_of_implementation_code_is_remembered(build_folder, memory_folder):
100+
api = FakeCodeplainAPI(
101+
conformance_fix_response=[
102+
FixConformanceTest.ISSUE_REASON_CODE_IMPLEMENTATION_CODE,
103+
{"app.py": "def add(a, b):\n return a + b + 1\n"},
104+
{"hypothesis": "off by one", "approach": "add one"},
105+
"prepared issue",
106+
]
107+
)
108+
ctx = make_conformance_context()
109+
render_context = make_render_context(api, build_folder, memory_folder, ctx)
110+
111+
outcome, _ = run_conformance_fix(render_context)
112+
113+
assert outcome == FixConformanceTest.IMPLEMENTATION_CODE_UPDATED
114+
assert len(ctx.implementation_code_fixes) == 1
115+
recorded_fix = ctx.implementation_code_fixes[0]
116+
assert recorded_fix["hypothesis"] == "off by one"
117+
assert recorded_fix["approach"] == "add one"
118+
assert list(recorded_fix["code_diff"]) == ["app.py"]
119+
assert "+ return a + b + 1" in recorded_fix["code_diff"]["app.py"]
120+
121+
122+
def test_consecutive_implementation_fixes_accumulate(build_folder, memory_folder):
123+
ctx = make_conformance_context()
124+
for attempt in range(2):
125+
api = FakeCodeplainAPI(
126+
conformance_fix_response=[
127+
FixConformanceTest.ISSUE_REASON_CODE_IMPLEMENTATION_CODE,
128+
{"app.py": f"def add(a, b):\n return a + b + {attempt + 1}\n"},
129+
{"hypothesis": f"hypothesis {attempt}", "approach": f"approach {attempt}"},
130+
"prepared issue",
131+
]
132+
)
133+
render_context = make_render_context(api, build_folder, memory_folder, ctx)
134+
run_conformance_fix(render_context)
135+
136+
assert [fix["hypothesis"] for fix in ctx.implementation_code_fixes] == ["hypothesis 0", "hypothesis 1"]
137+
138+
139+
def test_conformance_fix_without_summary_is_still_remembered(build_folder, memory_folder):
140+
api = FakeCodeplainAPI(
141+
conformance_fix_response=[
142+
FixConformanceTest.ISSUE_REASON_CODE_IMPLEMENTATION_CODE,
143+
{"app.py": "def add(a, b):\n return a + b + 1\n"},
144+
None,
145+
"prepared issue",
146+
]
147+
)
148+
ctx = make_conformance_context()
149+
render_context = make_render_context(api, build_folder, memory_folder, ctx)
150+
151+
run_conformance_fix(render_context)
152+
153+
assert ctx.implementation_code_fixes[0]["hypothesis"] is None
154+
assert ctx.implementation_code_fixes[0]["approach"] is None
155+
assert "app.py" in ctx.implementation_code_fixes[0]["code_diff"]
156+
157+
158+
def test_conformance_fix_of_conformance_tests_is_not_remembered(build_folder, memory_folder):
159+
api = FakeCodeplainAPI(
160+
conformance_fix_response=[
161+
FixConformanceTest.ISSUE_REASON_CODE_CONFORMANCE_TESTS,
162+
{"test_app.py": "def test_add(): pass\n"},
163+
{"hypothesis": "wrong assertion", "approach": "fix test"},
164+
"prepared issue",
165+
]
166+
)
167+
ctx = make_conformance_context()
168+
render_context = make_render_context(api, build_folder, memory_folder, ctx)
169+
170+
outcome, _ = run_conformance_fix(render_context)
171+
172+
assert outcome == FixConformanceTest.IMPLEMENTATION_CODE_NOT_UPDATED
173+
assert ctx.implementation_code_fixes == []
174+
175+
176+
def test_implementation_fix_with_no_files_is_not_remembered(build_folder, memory_folder):
177+
api = FakeCodeplainAPI(
178+
conformance_fix_response=[
179+
FixConformanceTest.ISSUE_REASON_CODE_IMPLEMENTATION_CODE,
180+
{},
181+
{"hypothesis": "nothing", "approach": "nothing"},
182+
"prepared issue",
183+
]
184+
)
185+
ctx = make_conformance_context()
186+
render_context = make_render_context(api, build_folder, memory_folder, ctx)
187+
188+
outcome, _ = run_conformance_fix(render_context)
189+
190+
assert outcome == FixConformanceTest.IMPLEMENTATION_CODE_NOT_UPDATED
191+
assert ctx.implementation_code_fixes == []
192+
193+
194+
def run_unit_tests_fix(render_context):
195+
return FixUnitTests().execute(render_context, {"previous_unittests_issue": "1 failed"})
196+
197+
198+
def test_unit_tests_fix_forwards_conformance_fixes(build_folder, memory_folder):
199+
api = FakeCodeplainAPI(unittests_fix_response={"test_app.py": "def test_add(): pass\n"})
200+
ctx = make_conformance_context()
201+
ctx.implementation_code_fixes.append(
202+
{"hypothesis": "off by one", "approach": "add one", "code_diff": {"app.py": "+ return a + b + 1"}}
203+
)
204+
render_context = make_render_context(api, build_folder, memory_folder, ctx)
205+
206+
outcome, _ = run_unit_tests_fix(render_context)
207+
208+
assert outcome == FixUnitTests.SUCCESSFUL_OUTCOME
209+
assert len(api.unittests_fix_calls) == 1
210+
assert api.unittests_fix_calls[0]["conformance_tests_fixes"] == ctx.implementation_code_fixes
211+
# The forwarded list is a copy, so later conformance fixes do not mutate what was sent.
212+
assert api.unittests_fix_calls[0]["conformance_tests_fixes"] is not ctx.implementation_code_fixes
213+
214+
215+
def test_unit_tests_fix_outside_conformance_phase_sends_no_fixes(build_folder, memory_folder):
216+
api = FakeCodeplainAPI()
217+
render_context = make_render_context(api, build_folder, memory_folder, None)
218+
219+
run_unit_tests_fix(render_context)
220+
221+
assert api.unittests_fix_calls[0]["conformance_tests_fixes"] is None
222+
223+
224+
def test_unit_tests_fix_in_conformance_phase_without_implementation_changes_sends_no_fixes(build_folder, memory_folder):
225+
api = FakeCodeplainAPI()
226+
render_context = make_render_context(api, build_folder, memory_folder, make_conformance_context())
227+
228+
run_unit_tests_fix(render_context)
229+
230+
assert api.unittests_fix_calls[0]["conformance_tests_fixes"] is None

0 commit comments

Comments
 (0)