PDFCLOUD-5839 Re-align tests with API#42
Merged
Conversation
✅ Deploy Preview for pdfrest-python ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
Had to close and open to get the tests to run, go figure. |
a3a16ca to
54e584d
Compare
datalogics-kam
requested changes
Jun 2, 2026
datalogics-kam
left a comment
Contributor
There was a problem hiding this comment.
Changes to the PDF/A support.
- Don't change the client interface to be open ended for
PdfAType. It should still strictly declare canonical types. Type hinting is a hint, so the user can still pass a lower casestr, and the low level validation will fix it. - I agree on the less strict validation before going to pdfRest, just avoid duplicating lists of values by using
get_args.
54e584d to
c4f7195
Compare
c4f7195 to
b7bb76a
Compare
datalogics-kam
left a comment
Contributor
There was a problem hiding this comment.
Review findings from Codex PR Review Auditor. I did not rerun live tests because the previously attached live server was incorrect; the comments below are based on the diff, policy files, and focused mocked/unit coverage.
datalogics-kam
requested changes
Jun 3, 2026
datalogics-kam
left a comment
Contributor
There was a problem hiding this comment.
The PDF/A work looks a lot better, but I ran the PR Review Auditor skill and it found a few things; please address those.
- Add sync and async MockTransport coverage for lowercase PDF/A output_type inputs in convert_to_pdfa. - Assert the outbound /pdfa request body normalizes those values to canonical PDF/A-2b. - Preserve the intent of the live tests with local unit coverage so request serialization regressions fail before live runs. Assisted-by: Codex
- Relax logo_opacity validation from gt=0 to ge=0 so callers can pass 0.0 through the public signature_configuration path. - Update the public signature-configuration type docs to reflect the supported [0, 1] opacity range. - Add sync and async unit coverage for request serialization and update payload bounds tests and live zero-opacity cases to exercise the public argument path instead of extra_body. - Keep zero-opacity support aligned across validation, documentation, unit tests, and live tests. Assisted-by: Codex
e82bfc5 to
f072f80
Compare
datalogics-kam
requested changes
Jun 5, 2026
datalogics-kam
left a comment
Contributor
There was a problem hiding this comment.
Looks good!
Could you add a version bump to 1.0.4 to this please?
datalogics-kam
approved these changes
Jun 9, 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
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.
PDFCLOUD-5839
Why this change
This branch closes two validation gaps in the SDK’s live and payload-level coverage.
First,
convert_to_pdfanow accepts case-insensitiveoutput_typestrings at validation time while keeping the public type surface opinionated around the canonicalPdfATypeliterals. That preserves IDE guidance and stricter typing for callers without forcing runtime inputs to already match the canonical case.Second, the live signing suite was treating
logo_opacity=0.0as invalid even though zero opacity is a valid boundary value. That left the branch without a test proving the lower bound is accepted.What changed
For PDF/A conversion, the payload validator now normalizes string
output_typevalues case-insensitively before enforcing the existingPdfATypeliteral. The canonical allowed values are derived fromget_args(PdfAType)so the validation logic does not maintain a second hard-coded list.The PDF/A live suite was updated to add explicit sync and async lowercase input coverage, while preserving the canonical uppercase coverage already driven from
PdfAType.For signing, the live tests now separate the invalid-opacity coverage from the zero-opacity boundary. Negative values remain the lower-bound failure case, and new sync and async live tests assert that
logo_opacity=0.0succeeds.Behavior changes
convert_to_pdfanow accepts inputs such as"pdf/a-2b"and normalizes them to the canonical PDF/A literal before request serialization. Invalid values still fail validation or server-side checks as before.The sign-PDF live suite now reflects the intended boundary behavior for
logo_opacity: values below0.0are rejected, while exactly0.0is treated as valid.Validation
Validated locally with:
uv run pytest -n auto --maxschedchunk 2 tests/test_convert_to_pdfa.pyuv run ruff check tests/test_convert_to_pdfa.pyuv run basedpyright tests/test_convert_to_pdfa.py tests/live/test_live_convert_to_pdfa.pyNot run locally:
tests/live/test_live_convert_to_pdfa.pytests/live/test_live_sign_pdf.pyThose live suites require pdfRest credentials and service access.
Risks and follow-ups
The main runtime change is intentionally narrow and limited to PDF/A
output_typenormalization. The larger risk is drift between live API behavior and the SDK’s expectations, which is why the added live coverage matters here.This branch does not add broader unit coverage for lowercase PDF/A inputs beyond the existing focused request/validation tests, and it does not rerun the full nox matrix as part of this iteration.