test(e2e): capture notebook cell errors in run_dbconnect diagnostics#2013
Open
rugpanov wants to merge 3 commits into
Open
test(e2e): capture notebook cell errors in run_dbconnect diagnostics#2013rugpanov wants to merge 3 commits into
rugpanov wants to merge 3 commits into
Conversation
*Why* When a run_dbconnect notebook/magic test times out waiting for an output file, `dumpNotebookDiagnostics` is supposed to reveal why the cell didn't emit. But its only error source is an Output-channel scrape via wdio-vscode-service, whose locator is `select[title="Tasks"]` — stale on the pinned VS Code (^1.86.0). On the Jul-14 CI run it threw "Can't call $$ on element with selector select[title=\"Tasks\"]" and returned zero channels, so the kernel's actual cell error/traceback was never captured. The only ground truth left was the .webm video, which showed the kernel now starts (the ipykernel fix from #2006 works) but a later cell (`%run`, or the `%sql`->`_sqldf` cell) either never runs or emits no output — undiagnosable from CI logs alone. *What* - Add `dumpActiveNotebookCells()`: reads the active notebook's cells, execution summaries (executionOrder/success), and output items via the VS Code API through `browser.executeWorkbench` — no DOM selector, so a cell error or a never-run cell is captured directly. - Add `dumpVscodeLogFiles()`: dumps VS Code's on-disk logs (extension host + Output-channel logs) resolved from `vscode.env.logUri`, another selector-independent source for the kernel/DBConnect error. - Wire both into `dumpNotebookDiagnostics` ahead of the existing Output-channel scrape, which is kept as best-effort (its failure is already caught and now clearly documented as a stale locator). - Diagnostics-only: runs solely on the failure path, changes no assertions or product code. *Verification* - `tsc --noEmit -p src/test/e2e/tsconfig.json`: 0 errors. - eslint + prettier on the file: clean. - (e2e itself runs only in the databricks-eng CI; this commit's purpose is to make the next CI run capture the cell error so the underlying cell-execution failure can be root-caused.) Co-authored-by: Isaac
Contributor
Author
|
🤖 Integration tests ❌ 5 of 35 test jobs failed for |
misha-db
approved these changes
Jul 14, 2026
*Why* The first diagnostics pass (dumpActiveNotebookCells) read `window.activeNotebookEditor`, which for the `%sql` magic test printed "<no active notebook editor>". That `.py` "Databricks notebook" (`# Databricks notebook source` + `# MAGIC %sql`) runs in an Interactive Window whose document is NOT the active notebook editor — the focused editor is the plain `.py` text editor — so the active-editor-only view missed exactly the failing cell we need to see. As a result the run captured the notebook.ipynb failure (cell #1 `%run` never ran) but still could not show why the `%sql`->`_sqldf` cell produced no output. *What* - Rename `dumpActiveNotebookCells` -> `dumpOpenNotebookCells` and iterate `vscode.workspace.notebookDocuments` (every open notebook doc), printing per document its uri + notebookType and per cell the executionOrder/success and output items. This captures the Interactive Window that runs the `.py` Databricks notebook. - Update the call site and comments accordingly. Still diagnostics-only, failure-path-only; no product or assertion changes. *Verification* - `tsc --noEmit -p src/test/e2e/tsconfig.json`: 0 errors. - eslint + prettier on the file: clean. - Purpose is to make the next e2e run surface the `%sql`/`_sqldf` cell error so the magic-comments failure can be root-caused with evidence. Co-authored-by: Isaac
Contributor
Author
|
🤖 Integration tests triggered for |
…defined" *Why* The previous commit's `dumpOpenNotebookCells` defined a named nested arrow (`const formatCell = (cell) => …`) inside the `browser.executeWorkbench` callback. WebdriverIO v9 transpiles the spec with tsx (esbuild, keepNames on), which rewrites a named binding as `const formatCell = __name(() => …, "formatCell")`. `executeWorkbench` serializes the callback via `.toString()` and evals it in the extension host, where the esbuild `__name` helper does not exist — so the callback threw "__name is not defined" and the whole dump was lost (CI run 29342709724 printed "could not read open notebook documents: Error: __name is not defined"). *What* - Inline the per-cell formatting directly into the `.map()` callbacks so there is no named nested binding for keepNames to wrap. Verified with the real tsx toolchain: `fn.toString()` of the inlined version contains no `__name`, whereas the named-const version emits `__name(...,"formatCell")`. - Add a comment warning that this callback must stay inline-arrow-only. - Behavior otherwise unchanged: still iterates `workspace.notebookDocuments` and prints per-cell executionOrder/success/outputs on the failure path only. *Verification* - Transpiled the actual file with the repo's esbuild+keepNames: 0 `__name` occurrences inside `dumpOpenNotebookCells`. - `tsc --noEmit -p src/test/e2e/tsconfig.json`: 0 errors; eslint + prettier: clean. Co-authored-by: Isaac
Contributor
Author
|
🤖 Integration tests triggered for |
Contributor
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Makes the
run_dbconnect.ucwse2e test capture why a notebook/magic cell fails, so the current flaky failures can be root-caused from CI logs instead of only from the.webmvideo.Background
The
ensureVenvHasKernelDepsfix (#2006) works — the kernel now starts and the notebook's first cell runs. But two failures remain (seen in run 29318458802):Notebook: Run Allcompletes cell 1, but the 2nd cell (%run "./hello.py") never executes (video shows cell[ ], status bar "Cell 1 of 2").%sql→_sqldfcell doesn't emit its output — fails on both Linux and Windows.We couldn't see the actual cell error because
dumpNotebookDiagnosticscrashed enumerating Output channels:That
select[title="Tasks"]locator (from wdio-vscode-service) is stale on the pinned VS Code^1.86.0, so the kernel's traceback never reached CI logs.What this does (diagnostics only — no product/assertion changes)
dumpActiveNotebookCells()— reads the active notebook's cells, execution summaries (executionOrder/success), and output items via the VS Code API throughbrowser.executeWorkbench. No DOM selector, so a cell error/traceback or a never-run cell is captured directly.dumpVscodeLogFiles()— dumps VS Code's on-disk logs (extension host + Output-channel logs) resolved fromvscode.env.logUri— another selector-independent source for the kernel/DBConnect error.Verification
tsc --noEmit -p src/test/e2e/tsconfig.json: 0 errors.%run/%sqlexecution failure can be diagnosed and fixed as a follow-up.This pull request and its description were written by Isaac.