Skip to content

Commit cc905ab

Browse files
rboni-dkclaude
andcommitted
refactor(runs): apply TG-1047 review feedback
- drop fabricated job_execution_id from the profile-history test mock; the run primary key is the job execution id, so assertions use run.id - on_delete_runs: only delete the job execution explicitly when no run row exists; cascade_delete already removes it via the FK chain otherwise Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent d051ca7 commit cc905ab

3 files changed

Lines changed: 11 additions & 11 deletions

File tree

testgen/ui/views/profiling_runs.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,8 @@ def on_delete_runs(job_execution_ids: list[str]) -> None:
307307
profiling_run = next(iter(select_profiling_runs_where(ProfilingRun.id == je_id)), None)
308308
if profiling_run:
309309
ProfilingRun.cascade_delete([str(profiling_run.id)])
310-
get_current_session().delete(job_exec)
310+
else:
311+
get_current_session().delete(job_exec)
311312
get_profiling_run_summaries.clear()
312313
Router().set_query_params({"page": 1})
313314
except Exception:

testgen/ui/views/test_runs.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,8 @@ def on_delete_runs(job_execution_ids: list[str]) -> None:
319319
test_run = next(iter(select_test_runs_where(TestRun.id == je_id)), None)
320320
if test_run:
321321
TestRun.cascade_delete([str(test_run.id)])
322-
get_current_session().delete(job_exec)
322+
else:
323+
get_current_session().delete(job_exec)
323324
get_test_run_summaries.clear()
324325
Router().set_query_params({"page": 1})
325326
except Exception:

tests/unit/mcp/test_tools_profile_history.py

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,6 @@ def _profile_row(
9696

9797
def _profiling_run(
9898
id_=None,
99-
job_execution_id=None,
10099
table_groups_id=None,
101100
status="Complete",
102101
profiling_starttime=None,
@@ -105,7 +104,6 @@ def _profiling_run(
105104
):
106105
run = MagicMock()
107106
run.id = id_ or uuid4()
108-
run.job_execution_id = job_execution_id or uuid4()
109107
run.table_groups_id = table_groups_id or uuid4()
110108
run.status = status
111109
run.profiling_starttime = profiling_starttime or datetime(2026, 5, 10, 12, 0)
@@ -283,7 +281,7 @@ def test_compare_profiling_runs_auto_baseline(
283281
mock_iss_type.select_where.return_value = []
284282

285283
with _patch_session([_je(), _je()]):
286-
result = compare_profiling_runs(str(target_run.job_execution_id))
284+
result = compare_profiling_runs(str(target_run.id))
287285

288286
assert "Profiling Run Comparison" in result
289287
assert "Target" in result and "Baseline" in result
@@ -298,7 +296,7 @@ def test_compare_profiling_runs_rejects_non_completed_target(mock_resolve, db_se
298296

299297
with _patch_session([_je(status=JobStatus.RUNNING)]):
300298
with pytest.raises(MCPUserError, match="Target run is in `Running` state"):
301-
compare_profiling_runs(str(target_run.job_execution_id))
299+
compare_profiling_runs(str(target_run.id))
302300

303301

304302
@patch("testgen.mcp.tools.profile_history.resolve_profiling_run")
@@ -308,7 +306,7 @@ def test_compare_profiling_runs_rejects_canceled_target(mock_resolve, db_session
308306

309307
with _patch_session([_je(status=JobStatus.CANCELED)]):
310308
with pytest.raises(MCPUserError, match="`Canceled`"):
311-
compare_profiling_runs(str(target_run.job_execution_id))
309+
compare_profiling_runs(str(target_run.id))
312310

313311

314312
@patch("testgen.mcp.tools.profile_history.resolve_profiling_run")
@@ -320,8 +318,8 @@ def test_compare_profiling_runs_rejects_cross_table_group(mock_resolve, db_sessi
320318
with _patch_session([_je()]):
321319
with pytest.raises(MCPUserError, match="same table group"):
322320
compare_profiling_runs(
323-
str(target_run.job_execution_id),
324-
str(baseline_run.job_execution_id),
321+
str(target_run.id),
322+
str(baseline_run.id),
325323
)
326324

327325

@@ -338,7 +336,7 @@ def test_compare_profiling_runs_auto_baseline_first_run(mock_resolve, db_session
338336

339337
with _patch_session([_je()]):
340338
with pytest.raises(MCPUserError, match="no earlier completed profiling run"):
341-
compare_profiling_runs(str(target_run.job_execution_id))
339+
compare_profiling_runs(str(target_run.id))
342340

343341

344342
@patch("testgen.mcp.tools.profile_history.HygieneIssue")
@@ -361,7 +359,7 @@ def test_compare_profiling_runs_identical_runs_renders_no_changes(
361359
mock_iss_type.select_where.return_value = []
362360

363361
with _patch_session([_je(), _je()]):
364-
result = compare_profiling_runs(str(target_run.job_execution_id))
362+
result = compare_profiling_runs(str(target_run.id))
365363

366364
assert "No changes between target and baseline" in result
367365

0 commit comments

Comments
 (0)