You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
feat(engineer-bot): require a live E2E repro for bug fixes (not unit-only) (#870)
* feat(engineer-bot): require a live E2E repro for bug fixes (not unit-only)
The bug-fix flow's red→green discipline doesn't guarantee the bug is *reproduced* —
only that the agent's test agrees with the agent's fix. On #868 (retry max→min) the
agent wrote/edited MOCKED unit tests to match its own wrong fix; they passed green,
but the change violated the real retry contract — caught only by pre-existing,
human-authored e2e tests. The engineer prompt here explicitly told the agent
"treat the unit suite as your only executable verification" — the opposite of the
sibling adbc-drivers/databricks bot, which REQUIRES a live E2E repro.
Port that discipline (adapted to Python/pytest/this connector):
Prompt (.bot/prompts/engineer/system.md):
- An E2E test (tests/e2e/, live warehouse) that reproduces the bug red and verifies
the fix green is REQUIRED; a mocked unit test alone is NOT sufficient. blocked (not
a unit-test substitute) if the behavior genuinely isn't e2e-observable.
- Test-first, reproduction is a HARD GATE (blocked if it can't fail-for-the-right-
reason after a focused effort).
- Do NOT rewrite an existing test's expectations to agree with the fix (the #868
failure mode); add a new failing test, and justify any existing-assertion change.
- Ground expected behavior in an external authority (issue/spec, or the JDBC
reference driver via context-repo) — not in the current connector code.
- Use a minimal, self-contained, -k-filtered e2e test (the bot job doesn't seed the
full fixture set).
Workflows (engineer-bot.yml author + engineer-bot-followup.yml run steps):
- Pass the 4 live-warehouse connection env vars the e2e suite needs
(DATABRICKS_SERVER_HOSTNAME / HTTP_PATH / CATALOG / USER), mirroring
code-coverage.yml. The jobs already run in `environment: azure-prod`, so the
secrets are in scope — they just weren't mapped into the run step.
Signed-off-by: eric-wang-1990 <e.wang@databricks.com>
Co-authored-by: Isaac
* ai: apply changes for #870 (2 review threads)
Addresses:
- #3600243617 at .github/workflows/engineer-bot.yml:194
- #3600243618 at .bot/prompts/engineer/system.md:48
Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
* ai: apply changes for #870 (1 review thread)
Addresses:
- #3600282324 at .bot/prompts/engineer/system.md:33
Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
* ai: apply changes for #870 (2 review threads)
Addresses:
- #3600313097 at .github/workflows/engineer-bot-followup.yml:155
- #3600313099 at .github/workflows/engineer-bot.yml:88
Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
* ai: apply changes for #870 (1 review thread)
Addresses:
- #3600339404 at .github/workflows/engineer-bot-followup.yml:107
Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
* ai: apply changes for #870 (1 review thread)
Addresses:
- #3600361719 at .bot/prompts/engineer/system.md:98
Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
* ai: apply changes for #870 (1 review thread)
Addresses:
- #3600385831 at .github/workflows/engineer-bot-followup.yml:158
Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
* ai: apply changes for #870 (1 review thread)
Addresses:
- #3600405525 at .github/workflows/engineer-bot-followup.yml:104
Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
* docs(bots): teach backend selection (Thrift/SEA/kernel) + realkernel tiers
Follow-up to the #870 review thread on --all-extras: the bot needs to know, per
issue, WHICH backend the bug is on and reproduce on that one — a Thrift bug won't
reproduce on a kernel connection, and a broad unit run with the real kernel wheel
present false-reds unless realkernel is deselected. That knowledge was tribal;
write it down.
- CONTRIBUTING.md: add a "Backends and test tiers" section — the three backends
(Thrift default / SEA `use_sea=True` / kernel `use_kernel=True`), where each
backend's tests live, that kernel is an opt-in extra, and the rule that
`realkernel` tests run in their own invocation (`-m "not realkernel"` for broad
runs), matching how CI (code-coverage.yml / code-quality-checks.yml) splits them.
- engineer/system.md: add step 0 — pick the backend the bug is on and reproduce
there; point to the CONTRIBUTING matrix.
- engineer-followup/system.md: correct the stale "do NOT run tests/e2e" line (the
followup job now has live creds via #870) and point at the same backend matrix.
Keeps --all-extras (both backends supported); the residual "prompt-discipline
only" risk the reviewer flagged is now backed by a documented, human-shared
convention plus explicit bot rules.
Signed-off-by: eric-wang-1990 <e.wang@databricks.com>
Co-authored-by: Isaac
* ai: apply changes for #870 (2 review threads)
Addresses:
- #3600996714 at .github/workflows/engineer-bot.yml:200
- #3601002346 at CONTRIBUTING.md:156
Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
* ai: apply changes for #870 (2 review threads)
Addresses:
- #3601040104 at .bot/prompts/engineer-followup/system.md:31
- #3601040111 at .bot/prompts/engineer/system.md:121
Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
---------
Signed-off-by: eric-wang-1990 <e.wang@databricks.com>
Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
Co-authored-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
Copy file name to clipboardExpand all lines: CONTRIBUTING.md
+27Lines changed: 27 additions & 0 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -144,6 +144,33 @@ The `PySQLStagingIngestionTestSuite` namespace requires a cluster running DBR ve
144
144
145
145
The suites marked `[not documented]` require additional configuration which will be documented at a later time.
146
146
147
+
#### Backends and test tiers
148
+
149
+
The connector has **three execution backends**, selected per connection. When you
150
+
reproduce or fix a bug, use the backend the bug is actually on — a Thrift bug won't
151
+
reproduce on a SEA or kernel connection, and vice versa:
152
+
153
+
| Backend | Select via (connect kwarg / `extra_params`) | Where its tests live |
154
+
| --- | --- | --- |
155
+
|**Thrift** (default) |*(nothing — the default path)*| the general `tests/e2e` suite (the `{}` parametrize case) and mocked `tests/unit`|
156
+
|**SEA** (Statement Execution API) |`use_sea=True`| the general `tests/e2e` suite (the `{"use_sea": True}` parametrize case, e.g. `tests/e2e/test_driver.py`) and mocked `tests/unit`|
157
+
|**Kernel** (Rust, optional) |`use_kernel=True`| the dedicated `tests/e2e/test_kernel_backend.py` / `test_kernel_tls.py`, plus the offline routing test `tests/unit/test_session.py -m realkernel`|
158
+
159
+
Notes that matter when running the suite:
160
+
161
+
-**Kernel is an opt-in extra**, not part of the default install. `use_kernel=True`
0 commit comments