Skip to content

PdfFile/test: CMake wiring + fixture investigation#105

Open
juliusknorr wants to merge 2 commits into
mainfrom
claude/enable-pdffile-tests
Open

PdfFile/test: CMake wiring + fixture investigation#105
juliusknorr wants to merge 2 commits into
mainfrom
claude/enable-pdffile-tests

Conversation

@juliusknorr

Copy link
Copy Markdown
Member

Summary

Migrates the qmake PdfFile/test/test.pro GoogleTest suite to a CMake/CTest target and documents the fixture situation.

  • New PdfFile/test/CMakeLists.txt: project(pdffile_test), CORE_ROOT_DIR = ../.., include(common.cmake), guard-added deps, add_core_gtest(NAME pdffile_test SOURCES test.cpp LIBS ... GTEST_MAIN GTEST_FILTER ...). test.cpp has no own main(), so GTEST_MAIN is used.
  • Registered in the top-level CMakeLists.txt if(EO_BUILD_TESTS) block, after the existing four suites.
  • TESTING.md updated with the full fixture catalogue and exact steps to fully enable.

Dependencies

test.pro's ADD_DEPENDENCY(UnicodeConverter, kernel, graphics, PdfFile, DjVuFile, ooxmlsignature) maps cleanly to existing CMake targets — no dependency lacks a CMake target, so no library porting is needed:

dep CMake target source
UnicodeConverter UnicodeConverter
kernel Common
graphics DesktopEditor/graphics/cmake
PdfFile PdfFile
DjVuFile DjVuFile
ooxmlsignature DesktopEditor/xmlsec/src

Gating decision

Registered live, but with one case excluded via GTEST_FILTER to keep CI green.

Every test case in test.cpp calls GTEST_SKIP() at the top except CPdfFileTest.EditPdfFromBase64, whose GTEST_SKIP() is commented out. That case calls LoadFromFile() (an ASSERT_TRUE on loading the missing test.pdf) and opens base64.txt, so it would hard-fail headless. It is excluded with GTEST_FILTER "-CPdfFileTest.EditPdfFromBase64". The suite then builds, links and runs cleanly (remaining cases report as skipped), so it gates the build/link without turning CI red.

Missing fixtures (none present anywhere in the repo)

Inputs: test.pdf, base64.txt, pdf.bin, pfx.pfx (PKCS#12, password 123456), test.djvu, changes.bin, test.jpeg (used by EditPdfSign; not in the original blocker list), and an resI/ directory of reference PNGs for ImgDiff.

Intermediate: ONLYOFFICEFORM.docxf (produced by GetMetaData, consumed by SetMetaData).

Outputs (generated at runtime, not required inputs): test2.pdf, test3.pdf, test_split.pdf, pdftemp/, resO/, resD/, fonts_cache/, resPdfBinToPng.png.

To fully enable

Commit the input fixtures above, stage them next to the test binary (the suite reads from NSFile::GetProcessDirectory(), so use WORKING_DIRECTORY plus a POST_BUILD copy_directory), drop the GTEST_FILTER, and un-GTEST_SKIP() the desired cases.

Build/CI note

There is no vcpkg toolchain locally (VCPKG_ROOT unset), so this could not be fully configured/built here. The PR's CI is the gate: configure with -DVCPKG_MANIFEST_FEATURES=tests -DEO_BUILD_TESTS=ON, build, then ctest --output-on-failure.

https://claude.ai/code/session_01TJrhgZT4PwYaBKWiUCXmGg


Generated by Claude Code

@juliusknorr juliusknorr requested a review from a team as a code owner June 12, 2026 18:12
@juliusknorr juliusknorr requested review from chrip and removed request for a team June 12, 2026 18:12
@juliusknorr juliusknorr force-pushed the claude/enable-pdffile-tests branch 2 times, most recently from 9f3a01c to 6acced3 Compare June 12, 2026 19:05
@juliusknorr juliusknorr requested review from a team and Aiiaiiio and removed request for a team June 16, 2026 19:43
@juliusknorr

Copy link
Copy Markdown
Member Author

🏓 for review. Will rebase and resolve conflicts before merging as every of the test prs conflicts

Add a CMakeLists.txt for PdfFile/test that mirrors the qmake test.pro
ADD_DEPENDENCY line (UnicodeConverter, kernel, graphics, PdfFile, DjVuFile,
ooxmlsignature) and builds the suite via add_core_gtest with GTEST_MAIN
(test.cpp has no own main()). All six dependencies map to existing CMake
library targets, so no dependency porting is required.

The suite's runtime fixtures (test.pdf, base64.txt, pdf.bin, pfx.pfx,
test.djvu, changes.bin, test.jpeg, resI/) are not committed to the repo.
Every test case self-skips except CPdfFileTest.EditPdfFromBase64, which would
hard-fail without test.pdf/base64.txt. That single case is excluded with
GTEST_FILTER so the suite builds, links and runs green; the remaining cases
report as skipped. Registered in the top-level EO_BUILD_TESTS block.

Update TESTING.md: move PdfFile/test to Done with the full fixture catalogue
(inputs vs intermediates vs outputs) and the exact steps to fully enable it.

Signed-off-by: Julius Knorr <jus@bitgrid.net>
Assisted-by: Claude Code:Opus 4.8
GoogleTest only defines EXPECT_HRESULT_SUCCEEDED/ASSERT_HRESULT_SUCCEEDED on
Windows. The PdfFile suite uses them, so it failed to compile on the Linux CI
build with "use of undeclared identifier 'EXPECT_HRESULT_SUCCEEDED'". Add
portable definitions (HRESULT success is >= 0 on all platforms) so the suite
compiles. The affected cases still GTEST_SKIP() at runtime pending fixtures.

Signed-off-by: Julius Knorr <jus@bitgrid.net>
Assisted-by: Claude Code:Opus 4.8
@juliusknorr juliusknorr force-pushed the claude/enable-pdffile-tests branch from 6acced3 to b994641 Compare June 30, 2026 20:36
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.

2 participants