temp: add Windows reproduction for export-dir on illegal notebook name#5655
Draft
radakam wants to merge 4 commits into
Draft
temp: add Windows reproduction for export-dir on illegal notebook name#5655radakam wants to merge 4 commits into
radakam wants to merge 4 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: bba3ce7
20 interesting tests: 13 SKIP, 7 KNOWN
Top 22 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 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.
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.Closes DECO-27435 and fixes issue #5171.
Tests
export-dir-illegal-filename/, Windows-gated): export now skips-with-warning and completes; golden captured from a real Windows CI run.export_dir_windows_test.go): asserts the narrow contract — onlyERROR_INVALID_NAMEskips;ErrPermission/ErrNotExist/generic/nildo not.