Bump deps to latest, edition 2024, and get integration tests passing#1
Merged
Conversation
Dependency + edition bumps:
- edition 2021 -> 2024 (Rust 1.85+)
- chrono, clap, serde, serde_json, tempfile, thiserror, tracing,
tracing-subscriber, reqwest, tokio, tokio-test all bumped to latest stable
Test infrastructure:
- Added src/lib.rs so integration tests can drive the server in-process
via a thread + OnceLock instead of spawning `cargo run` as a child
(eliminates the leaked child-process problem from the prior review).
- Added a `tester` stage to the existing Dockerfile that layers
libreoffice-writer, libreoffice-calc, and fonts on top of `builder`
so cargo test can actually convert documents.
- Rewrote tests to address the prior review:
* Drop the 200 MB large_file.docx fixture; generate the oversized
payload in-memory.
* Use valid xlsx bytes under a .docx filename to actually exercise
the ContentMismatch path.
* Update assertions to match the validator's real error strings.
* Drop the rate-limiting test (no rate limiter exists).
Bug fixes uncovered while getting tests green:
- http_server: NamedTempFile::with_suffix(...) so LibreOffice can
autodetect the input format. document_load hangs silently on a
path with no extension.
- tests: set .mime_str("application/octet-stream") on every binary
multipart::Part so rouille's old multipart 0.18 doesn't treat
Content-Type-less parts as text and fail UTF-8 decoding.
Dockerfile CMD greps for `test result: ok` because LibreOfficeKit's
atexit cleanup emits "Unspecified Application Error" and forces the
test binary to exit 1 even when libtest reports success.
GitHub Actions workflow runs on PRs to main and pushes to main: - fmt: dtolnay/rust-toolchain on ubuntu-latest, `cargo fmt --all -- --check` - clippy: builds the Dockerfile `tester` stage and runs `cargo clippy --all-targets -- -D warnings` inside it (libreofficekit-dev is needed to compile libreoffice-rs) - test: builds the same `tester` image and runs it with --cap-add=SYS_NICE Drive-bys to satisfy `-D warnings`: - src/http_server.rs: replace `|e| ConversionError::Io(e)` with the variant constructor directly - src/main.rs: drop the `let _ =` binding around `tracing_subscriber::fmt::init()` - tests: drop unused `expected_mime`, collapse nested `if let`+`if` into `if let .. && ..`, replace `expect(&format!(...))` with `unwrap_or_else(|_| panic!(...))`, drop `&` on `format!(...)` arguments to reqwest's generic `IntoUrl` methods Dockerfile tester stage installs `clippy` and `rustfmt` rustup components (stripped from the slim base) and applies cargo fmt across the tree.
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
1 participant
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
2021→2024(requires Rust 1.85+).src/lib.rs+src/main.rsso integration tests can run the HTTP server in-process via a thread (no morecargo runchild process to leak).testerstage to the existingDockerfilethat layerslibreoffice-writer,libreoffice-calc, and fonts on top ofbuilder—libreofficekit-devalone provides core but no format modules, so docx/xlsx loads were hanging.Bug fixes uncovered while getting tests green
http_server.rs:NamedTempFile::with_suffix(...)for input/output temp files. LibreOffice'sdocument_loadhangs silently on extensionless paths (/tmp/.tmpXXXXXX) — it relies on the extension for format autodetection.tests/integration_tests.rs:.mime_str("application/octet-stream")on every binarymultipart::Part. rouille's oldmultipart0.18 has anis_text()that defaults totruewhen a part has no Content-Type, so binary docx bytes were being read as UTF-8 and parsing died.Dockerfile note
The
testerCMDgreps^test result: ok\..*0 failedfrom the captured output. LibreOfficeKit's atexit cleanup emitsUnspecified Application Errorand forces the test binary to exit 1 even after libtest reportstest result: ok. This is the pragmatic workaround; if we ever want it nicer, options arelibtest_mimicwith a customharness = false, or going back to a subprocess server.The container also needs
--cap-add=SYS_NICEto run, otherwise LO's thread-priority calls trigger a glibc abort:Test plan
docker build --target tester -t docoxide-tester .succeedsdocker run --rm --cap-add=SYS_NICE docoxide-tester→ 16/16 tests pass, container exit code 0docker build(default target = runtime) still produces a working production image