chore: move cost tests into separate file

[ysmolski]

They have grown and will grow some more.
It is better to have them separated.

Updates ENG-8739

Summary by CodeRabbit

• Tests

  • Added a comprehensive suite validating cost estimation across many GraphQL scenarios (nested lists, slicing, fractional multipliers, interfaces/unions, argument weights, empty lists, and actual vs. estimated costs).

• Refactor

  • Centralized and standardized the execution test harness to simplify setup, consistent validations, fixture comparisons, and configurable cost/error checks.

[coderabbitai]

📝 Walkthrough

Walkthrough

Added a large execution engine cost-estimation test suite and refactored the execution engine tests to replace a gRPC-specific factory with a generalized test harness (runExecutionTest) that centralizes test setup, execution, and assertions.

Changes

Cohort / File(s)	Summary
Cost Estimation Tests execution/engine/execution_engine_cost_test.go	New comprehensive Go test file exercising execution engine cost estimation across weights, argument weights, negative weights, interfaces/unions, fragments, list sizing/slicing, nested and deeply nested lists, fractional multipliers, and actual vs estimated cost comparisons.
Test Infrastructure Refactoring execution/engine/execution_engine_test.go	Removed gRPC-specific factory and introduced a generalized test harness: runExecutionTest, plus wrappers runWithAndCompareError and runWithoutError. Centralized test orchestration, options for skips, JSON formatting, fixture comparisons, and cost assertions; replaced many per-test wrappers with the new harness.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: moving cost tests from the main execution engine test file into a new separate test file.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch yury/move_costs_test

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

execution/engine/execution_engine_cost_test.go (1)

817-825: Known-wrong union cost behavior is being locked in

These assertions intentionally encode values the comments call incorrect. That can make future algorithm fixes look like regressions. Consider marking these as transitional (with linked issue IDs) and separating “current behavior” from “target behavior” expectations.

Also applies to: 876-883

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@execution/engine/execution_engine_test.go`:
- Around line 142-149: The test currently skips assertions when
testCase.expectedEstimatedCost or testCase.expectedActualCost equals 0, which
hides valid zero-cost expectations; change the logic to always call
operation.EstimatedCost() and operation.ActualCost() and assert equality with
testCase.expectedEstimatedCost and testCase.expectedActualCost (use
require.Equal(t, testCase.expectedEstimatedCost, gotCost) and require.Equal(t,
testCase.expectedActualCost, gotActualCost)) instead of wrapping them in the
existing if checks so zero values are verified.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c0eb3137-5943-422b-b640-590de4f85948

📥 Commits

Reviewing files that changed from the base of the PR and between 78ea8ff and 593908d.

📒 Files selected for processing (2)

• execution/engine/execution_engine_cost_test.go
• execution/engine/execution_engine_test.go

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (1)

execution/engine/execution_engine_test.go (1)

143-150: ⚠️ Potential issue | 🟠 Major

Zero-value cost expectations are still skipped

Line 143 and Line 148 still skip assertions when expected cost is 0, so zero-cost scenarios remain unverified.

💡 Suggested fix

-		if testCase.expectedEstimatedCost != 0 {
+		if opts.computeCosts || testCase.expectedEstimatedCost != 0 {
 			gotCost := operation.EstimatedCost()
 			require.Equal(t, testCase.expectedEstimatedCost, gotCost)
 		}
 
-		if testCase.expectedActualCost != 0 {
+		if opts.computeCosts || testCase.expectedActualCost != 0 {
 			gotActualCost := operation.ActualCost()
 			require.Equal(t, testCase.expectedActualCost, gotActualCost)
 		}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@execution/engine/execution_engine_test.go` around lines 143 - 150, The test
currently skips assertions when expected costs are zero; remove the conditional
guards and always assert the returned costs against
testCase.expectedEstimatedCost and testCase.expectedActualCost so zero-value
expectations are verified. Locate the assertions around
operation.EstimatedCost() and operation.ActualCost() (references:
operation.EstimatedCost, operation.ActualCost, testCase.expectedEstimatedCost,
testCase.expectedActualCost) and replace the if-blocks with unconditional
require.Equal checks that compare the expected value to gotCost/gotActual.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@execution/engine/execution_engine_test.go`:
- Around line 120-124: The fixture-assertion branch returns before validating
the execution error, so tests can ignore unexpected errors; update the branch
that handles testCase.expectedFixture to first check the execution error
variable (err) against testCase.withError (or the appropriate expected error
flag) and assert it accordingly, then proceed to create the goldie instance (g
:= goldie.New(...)) and call g.Assert with []byte(actualResponse) only after the
error assertion; ensure both success and expected-error cases are covered before
the return.

---

Duplicate comments:
In `@execution/engine/execution_engine_test.go`:
- Around line 143-150: The test currently skips assertions when expected costs
are zero; remove the conditional guards and always assert the returned costs
against testCase.expectedEstimatedCost and testCase.expectedActualCost so
zero-value expectations are verified. Locate the assertions around
operation.EstimatedCost() and operation.ActualCost() (references:
operation.EstimatedCost, operation.ActualCost, testCase.expectedEstimatedCost,
testCase.expectedActualCost) and replace the if-blocks with unconditional
require.Equal checks that compare the expected value to gotCost/gotActual.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: dd474333-e8ba-4f78-a0d1-de42c18397e5

📥 Commits

Reviewing files that changed from the base of the PR and between 593908d and eac23c1.

📒 Files selected for processing (1)

• execution/engine/execution_engine_test.go