Skip to content

test(ui): extend native opening and continuation regression coverage - #1764

Closed
LabDennis wants to merge 1 commit into
storytold:mainfrom
LabDennis:fix/async-open-continuations
Closed

LabDennis wants to merge 1 commit into
storytold:mainfrom
LabDennis:fix/async-open-continuations

Conversation

@LabDennis

@LabDennis LabDennis commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Upstream main includes the production fix for native open actions and password prompts (b5467d9, #1914). This PR is synchronized with that implementation and retains only additional regression coverage.

The remaining tests cover one-shot picker actions through plain/encrypted opening and cancellation, actions after text/image conversion (including failures), and startup intake while a local password retry is pending. The reload integration test pins its language to English.

Validation: all 14 open_worker_tests pass; formatting, asset attribution, dependency layers and diff checks pass. The latest upstream watcher change is also included (its two regression tests pass in the responsive-opening branch). No production-code or maintainer-roadmap changes remain relative to main. Unrelated existing CI failures are outside this update.

The branch is now based on upstream 9931329b9fae. History: 4 → 1 commits. The rewritten file tree was checked against the previous PR plus current upstream, preserving prior integration resolutions.

Scoped Windows regression checks: 21 passed, 0 ignored, zero failures. Formatting, locked Cargo metadata, asset and layer checks passed. This is scoped validation, not a new full-workspace or cross-platform CI claim.

@FredApps FredApps left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at e15084d against 7437bde. This isolates the continuation and startup/password-ordering fixes I raised on #1634, without pulling in its opening-performance refactor.

I checked the command handoff through background preparation, conversion and password retry, and the startup queue guard while preparation is pending. The new barrier test exercises the ordering directly, and the integration waits retain the behavioral assertions and background path. No blocking issue found in this scope.

Independent Linux validation at the exact head, Rust 1.99 with craft-fonts: 271 passed, 0 failed, 4 existing ignored. Breakdown: open_worker_tests 4; commands 22; document_links 30; editing 117 (4 ignored); pickers 35; recovery 11; reload 7; ui 45.

Commands: cargo test --offline --locked --profile low-disk -p pdfcraft-ui-egui --lib open_worker_tests, followed by the commands/document_links/editing/pickers/recovery/reload/ui integration binaries with --no-fail-fast. No assertions were changed or tests switched to inline execution for this validation.

The exact-head GitHub checks also show Ubuntu, Windows, WASM and dependency checks passing; fuzz was skipped. I did not independently rerun the full workspace or FreeBSD. #1581 and #1672 can be rebased and rechecked once this fix lands; I have kept their feature branches separate.

@jcl-bit

jcl-bit commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

I hit the red main while verifying an unrelated packaging branch, so I bisected it in case it helps whoever reviews this. Last green main CI is 0a09864, and the first bad commit is 55f76ab (#1493). Its parent ecc4a30 is green in all seven ui-egui test binaries I ran (commands, document_links, editing, pickers, recovery, ui, viewer). At 55f76ab the same run has 15 failures: commands 2, document_links 2, editing 2, pickers 3, recovery 5 and ui 1 (dropping_a_pdf_on_the_window_opens_it). Since #1493 a running app reads and prepares an opened file on the open worker, so any test that opens a file and asserts after a fixed number of frames can run before the worker finishes.

That makes this PR the one that covers the whole set. #1897 only fixes the two commands.rs tests, #1729 only the two drop tests, and #1681 only the tool-chosen-with-no-document case, so merging any of those alone leaves main red. This PR touches all five failing binaries and its Ubuntu and Windows test jobs are green. If a maintainer is choosing which one to land to get main green again, this looks like the right one.

@LabDennis LabDennis changed the title M1: Restore deferred tools and startup ordering after background opens test(ui): extend native opening and continuation regression coverage Oct 14, 2026
@FredApps

Copy link
Copy Markdown
Contributor

Rechecked exact head d40e5c9ed68527c7f421c0ed66b5704fa4e57345 after the upstream sync. The production continuation and startup-ordering fix has landed through #1914; the remaining diff here is additional worker regression coverage and the reload-test language pin. No new blocker found in that diff.

Independent Linux testing on this head: 14 worker tests plus 271 tests across commands, document_links, editing, pickers, recovery, reload and ui passed; 4 existing tests ignored, zero failures. Used the pinned dependencies and craft-fonts, with no source edits or weakened assertions. This is targeted validation, not a full workspace or native GUI claim.

LabDennis added a commit to LabDennis/pdfcraft that referenced this pull request Oct 14, 2026
Retain the complete reviewed change, including prior integration resolutions,
on current upstream.

M1: Preserve tools and startup ordering during background opens

Carry deferred commands through PDF preparation, password retries and successful conversion. Keep startup bytes queued until local opening completes. Replace frame-count assumptions with completion-aware UI tests and pin both behavior regressions with failing-before tests.
@LabDennis
LabDennis force-pushed the fix/async-open-continuations branch 2 times, most recently from 9fea5d3 to ec8f2a3 Compare October 14, 2026 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants