test: expand PS1PLY and PS1TMD coverage

[fernandotonon]

Summary

• Add 9 PS1TMD tests for previously uncovered primitive modes (0x20 flat lit, 0x24/0x34 lit textured tris, 0x2d/0x3c/0x3d textured quads), export guard rails (null entity, empty mesh), and truncated primitive streams.
• Add 7 PS1PLY tests for quad-face import/export, importPsyqPlyWithFaceColors, unlit _nl material suffix via face materials, and exportPsyqPlyFromEntity error handling.

Test plan

• xvfb-run ./build_local/bin/UnitTests --gtest_filter="PS1PLY*:PS1TMDTest.*" — 39 passed locally
• CI unit-tests-linux

Made with Cursor

Summary by CodeRabbit

• Tests
  • Expanded test coverage for PS1 PLY and TMD 3D model import/export, including quad-face handling, vertex color application, and material generation validation.
  • Enhanced error handling and edge case verification for mesh processing, export operations, and invalid input scenarios.

[coderabbitai]

Warning

Rate limit exceeded

@fernandotonon has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 19 minutes and 26 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3c7d7d42-d1f8-4f99-8ecb-158ee29c9bbf

📥 Commits

Reviewing files that changed from the base of the PR and between b12793a and 4a9bdd4.

📒 Files selected for processing (2)

• src/PS1/PS1PLY_test.cpp
• src/PS1/PS1TMD_test.cpp

📝 Walkthrough

Walkthrough

This PR adds 615 lines of test coverage to PS1PLY_test.cpp and PS1TMD_test.cpp, introducing new test helpers and comprehensive import/export validation for PS1 PLY and TMD formats, including quad triangulation, face colors, error handling, and roundtrip preservation.

Changes

PS1 PLY and TMD Import/Export Test Coverage

Layer / File(s)	Summary
PLY Quad Helper and Import Tests src/PS1/PS1PLY_test.cpp	Introduces writeQuadPsyqPly helper to generate test PLY files with quad faces. New import tests validate quad-to-triangle expansion, face-color application with color element verification, face-color vector size validation, and unlit material naming via _tex0_nl suffix detection.
PLY Export Error Handling and Roundtrip Tests src/PS1/PS1PLY_test.cpp	Tests PLY export error cases: null entity returns false with error string, unwritable path returns false with error string. Roundtrip test confirms a quad-face PLY exports with quad preservation (single face line with quad indicator) rather than triangulation.
TMD Blob Generator Helpers src/PS1/PS1TMD_test.cpp	Adds minimal TMD blob generators for flat-lit triangles, lit textured triangles, Gouraud textured triangles, and textured/no-light quad variants. Includes buffer layout constants and direct binary writes for reproducible test geometry.
TMD Import Helper and Mode Tests src/PS1/PS1TMD_test.cpp	Introduces importTmdBlobAndExpectTris helper for temporary file-based TMD import validation with vertex count checks. Tests import across modes 0x20, 0x24, 0x34, 0x2d, 0x3c, and 0x3d with UV-presence assertion for 0x24 and expected triangulation behavior.
TMD Export and Negative Test Cases src/PS1/PS1TMD_test.cpp	Tests exportEntity failure cases: returns false for null entities and meshes with no submeshes. Negative import test confirms importTmd returns false when primitive stream is truncated.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• fernandotonon/QtMeshEditor#500: The new PLY tests for importPsyqPlyWithFaceColors and unlit material-name suffix (_tex0_nl) directly validate the lit/unlit and FaceMaterial handling.
• fernandotonon/QtMeshEditor#454: The expanded PS1PLY_test.cpp coverage for Psy-Q PLY import/export (quad triangulation, face colors, unlit material naming) validates the PLY/MAT/RSD PLY import/export functionality.
• fernandotonon/QtMeshEditor#394: The new PS1 TMD unit tests directly exercise the PS1 .tmd import/export functionality and error handling paths.

Poem

🐰 A rabbit's ode to test coverage bright,
New quadface quests and TMD light,
PLY helpers hop and TMD blob setters sing,
Error cases caught with every test we bring,
Import, export, roundtrip—all checks align! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: expanding test coverage for PS1PLY and PS1TMD modules.
Description check	✅ Passed	The description provides comprehensive technical details and test validation, but lacks proper alignment with the template structure which requires distinct sections.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch test/ps1-ply-tmd-coverage

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/PS1/PS1PLY_test.cpp`:
- Around line 795-800: The test
PS1PLYOgreTest::ExportPsyqPlyFromEntity_NullEntityReturnsFalse uses a hardcoded
Unix path "/tmp/x.ply"; change the test to construct a platform-agnostic
temporary path (e.g., using QDir::tempPath() combined with a filename) when
calling PS1PLY::exportPsyqPlyFromEntity(nullptr, ...), so the test runs on
Windows, Linux and macOS while keeping the same assertions.
- Around line 802-818: The test uses a Linux-only path in badPath; replace that
with a cross-platform unwritable target and update the test to create it at
runtime (modify TEST_F
PS1PLYOgreTest::ExportPsyqPlyFromEntity_UnwritablePathSetsError). Use
QTemporaryDir/QTemporaryFile (or create a temp dir) and then make the target
unwritable (e.g., create a temp file and call QFile::setPermissions to remove
write permission, or pass the temp directory path as the export target) and
assign that path to badPath before calling PS1PLY::exportPsyqPlyFromEntity; this
ensures the export fails and err is set on Windows/macOS/Linux without
hardcoding /proc.

In `@src/PS1/PS1TMD_test.cpp`:
- Around line 1215-1260: The new Ogre-dependent tests (e.g.,
ImportMode24LitTexturedTriangle_HasUvs, ImportMode20FlatLitTriangle,
ImportMode34GouraudTexturedTriangle, ImportMode2dNoLightTexturedQuad,
ImportMode3cGouraudTexturedQuad, ImportMode3dGouraudTexturedQuadNoLight) must
fail fast when Ogre prerequisites are missing; update the test fixture SetUp to
assert prerequisites by calling ASSERT_TRUE(tryInitOgre()) and
ASSERT_TRUE(canLoadMeshFiles()) (instead of silently skipping), so add those two
ASSERT_TRUE checks in the fixture SetUp used by these TEST_F cases (where
tryInitOgre and canLoadMeshFiles are defined) to ensure CI fails loudly when
mesh-file support is unavailable.
- Around line 1233-1236: The test dereferences getSubMesh(0)->vertexData without
ensuring the mesh has any submeshes; before accessing mesh->getSubMesh(0) assert
mesh->getNumSubMeshes() > 0 (or ASSERT_NE(mesh->getNumSubMeshes(), 0)) to guard
the access. In the PS1TMD_test.cpp block around the PS1TMD::importTmd(...)
usage, add an assertion on mesh->getNumSubMeshes() and only then grab const
Ogre::VertexData* vd = mesh->getSubMesh(0)->vertexData to prevent crashes when
no submeshes exist.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8fd7b23e-3605-485b-9041-cc01c349be34

📥 Commits

Reviewing files that changed from the base of the PR and between d01cc8a and b12793a.

📒 Files selected for processing (2)

• src/PS1/PS1PLY_test.cpp
• src/PS1/PS1TMD_test.cpp

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud