temp: add Windows reproduction for export-dir on illegal notebook name#5655
Open
radakam wants to merge 5 commits into
Open
temp: add Windows reproduction for export-dir on illegal notebook name#5655radakam wants to merge 5 commits into
radakam wants to merge 5 commits into
Conversation
Reproduces #5171: `workspace export-dir` aborts on Windows when a notebook name contains a ':' (e.g. an untitled "New Notebook <timestamp>"), because the colon is illegal in a local Windows filename and os.Create fails. The name is legal on Linux/macOS, so the test is gated to Windows via GOOS. output.txt is intentionally omitted: the golden cannot be generated on a non-Windows machine, so the Windows CI leg will surface the real output to commit verbatim.
Collaborator
Integration test reportCommit: 50a000c
20 interesting tests: 13 SKIP, 7 RECOVERED
Top 21 slowest tests (at least 2 minutes):
|
Captured verbatim from the Windows CI run of the test added in the previous commit. Documents the current (buggy) behavior of #5171: export-dir aborts on Windows when a notebook name contains ':'.
A workspace object name can be illegal as a local filename (e.g. the ':' in an untitled "New Notebook 2026-05-04 13:54:24" on Windows), which made os.Create fail and abort the entire export. Skip such files with a warning and continue, matching the existing skip paths for non-exportable and oversized objects. Detection uses a build-tagged helper checking the Windows ERROR_INVALID_NAME code; on other platforms no filename character (other than '/' and NUL, which cannot appear in a workspace object name) is illegal, so it is a no-op. Flips the Windows-gated acceptance test from asserting the abort to expecting skip-with-warning. Fixes #5171.
Locks in the narrow contract that only the Windows ERROR_INVALID_NAME error is treated as skippable, while permission, not-exist, generic, and nil errors are not. The acceptance test only exercises the positive (invalid-name) path, so this guards against accidentally broadening the check and silently dropping files on real I/O failures.
| # This reproduces databricks/cli#5171, which only manifests on Windows: a ':' in | ||
| # the notebook name is a legal filename character on Linux/macOS but illegal on | ||
| # Windows, so os.Create rejects the local target path there. | ||
| [GOOS] |
Contributor
There was a problem hiding this comment.
lets run this test on all operating systems. You can redirect the stderr / stdout to out.(os).txt to capture the operating system differences in teh same PR.
Contributor
Author
There was a problem hiding this comment.
Done -- runs on all three OSes now, with export-dir output redirected to out.(os).txt; output.txt keeps one OS-independent line so there's no empty golden. One note: it seems that the runner had no per-OS golden support, so a plain out.windows.txt would fail as "Missing output file" on linux/macos if I understand it correctly. I extended preparePathFilter to compare only the current OS's file and skip the rest, and added a fake WorkspaceList handler so the notebook is actually listed and exported. Is this what you had in mind?
The test reproducing #5171 was gated to Windows only because a ':' in the notebook name is illegal as a local filename on Windows but legal on Linux/macOS, so export-dir skips it there and exports it everywhere else. Run it on every OS and capture the divergent output in out.<goos>.txt. Teach the acceptance runner to compare only the current OS's out.<goos>.txt and skip the others (extending the existing variant-skip machinery), since otherwise the non-current goldens would fail as "Missing output file". Add a fake WorkspaceList handler so the imported notebook is listed and actually exported, replacing the hardcoded [[Server]] list stub.
shreyas-goenka
approved these changes
Jun 19, 2026
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
3 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.
Why
A UI-created notebook is named like
New Notebook 2026-05-04 13:54:24. The:is legal on Linux/macOS but illegal on Windows, soos.Createfails withERROR_INVALID_NAMEand the whole recursive export aborts on that one file.Changes
export_dir.go: skip such a file with a warning and continue, instead of aborting. Only this specific error is skipped — permission/disk/missing-path errors still propagate (so we never silently drop files on real failures).export_dir_windows.go/export_dir_other.go) to avoiderr.Error()string-matching and stay cross-platform;ERROR_INVALID_NAMEhas no portable Go sentinel.libs/testserver: added a fakeWorkspaceListhandler so the imported notebook is actually listed and exported.Changes to acceptance harness
out.<goos>.txt;output.txtholds only an OS-independent header line.preparePathFilternow compares only the current OS'sout.<goos>.txtand skips the others. Previously every committedout*file was required on every OS, so per-OS goldens weren't possible.Closes DECO-27435 and fixes issue #5171.
Tests
export-dir-illegal-filename/): runs on all OSes; export skips-with-warning on Windows (out.windows.txt, captured from a real Windows CI run) and completes-with-export on Linux/macOS (out.linux.txt/out.darwin.txt).export_dir_windows_test.go): asserts the narrow contract — onlyERROR_INVALID_NAMEskips;ErrPermission/ErrNotExist/generic/nildo not.