Use CPPUNIT_ASSERT_EQUAL instead of CPPUNIT_ASSERT

[bukka]

This shows the actual incorrect value

Summary by CodeRabbit

• Tests
  • Standardized assertion style across test suite for clearer, more consistent pass/fail reporting.
  • Improved readability of test failures by explicitly showing expected vs. actual values.
  • No changes to product behavior; tests only.

[coderabbitai]

Walkthrough

Updated test assertions in ObjectTests.cpp from equality comparisons using CPPUNIT_ASSERT(rv == CKR_XXX) to CPPUNIT_ASSERT_EQUAL(CKR_XXX, rv). No test logic or control flow changed.

Changes

Cohort / File(s)	Summary
Test assertion normalization src/lib/test/ObjectTests.cpp	Replaced CPPUNIT_ASSERT(rv == CKR_) with CPPUNIT_ASSERT_EQUAL(CKR_, rv) across multiple test cases, affecting OK and error codes; no functional logic changes.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

I twitch my nose at passing code,
Swap asserts on the testing road.
Expected left, actual right—
Clearer prints in moonlit night.
Thump-thump! My paws approve the zeal,
Tiny change, more readable feel. 🐰✨

✨ Finishing Touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

• Visit our Status Page to check the current availability of CodeRabbit.
• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[kalvdans]

Great, although I think it reads better with the arguments swapped, "assert that this variable under test rv equals this golden reference constant CKR_OK"

[coderabbitai]

Actionable comments posted: 0

🔭 Outside diff range comments (1)

src/lib/test/ObjectTests.cpp (1)

705-953: Fix legacy CPPUNIT_ASSERT(rv == CKR_*) usages across the test suite

We ran the provided regex search and uncovered numerous occurrences of the old-style assertions in tests under src/lib/test and src/lib/session_mgr/test. To ensure consistency and clear diagnostics, please replace every instance of:

• CPPUNIT_ASSERT(rv == CKR_<CODE>);
• CPPUNIT_ASSERT(CKR_<CODE> == rv);

with the new pattern:

CPPUNIT_ASSERT_EQUAL(CKR_<CODE>, rv);

Files that need updating (non-exhaustive list):

• src/lib/test/ForkTests.cpp
• src/lib/test/UserTests.cpp
• src/lib/test/SessionTests.cpp
• src/lib/test/TokenTests.cpp
• src/lib/test/InitTests.cpp
• src/lib/test/SignVerifyTests.cpp
• src/lib/test/RandomTests.cpp
• src/lib/test/InfoTests.cpp
• src/lib/test/SymmetricAlgorithmTests.cpp
• src/lib/test/DigestTests.cpp
• src/lib/test/AsymWrapUnwrapTests.cpp
• src/lib/test/DeriveTests.cpp
• src/lib/test/AsymEncryptDecryptTests.cpp
• src/lib/session_mgr/test/SessionManagerTests.cpp

Once updated, re-run:

rg -n --pretty 'CPPUNIT_ASSERT\s*\(\s*(rv\s*==|==\s*rv)\s*CKR_'

to confirm no legacy asserts remain.

🧹 Nitpick comments (2)

src/lib/test/ObjectTests.cpp (2)

1476-1477: Optional: extend the pattern to other equality checks for better diffs

There are still several equality assertions using raw comparison within CPPUNIT_ASSERT (e.g., “4 == ulObjectCount”, “objectSize == CK_UNAVAILABLE_INFORMATION”, memcmp(...) == 0). Converting the plain equality ones to CPPUNIT_ASSERT_EQUAL will produce clearer failure output for the actual values.

For example:

-CPPUNIT_ASSERT(4 == ulObjectCount);
+CPPUNIT_ASSERT_EQUAL((CK_ULONG)4, ulObjectCount);

-CPPUNIT_ASSERT(objectSize == CK_UNAVAILABLE_INFORMATION);
+CPPUNIT_ASSERT_EQUAL((CK_ULONG)CK_UNAVAILABLE_INFORMATION, objectSize);

Leave memcmp-based checks as-is unless you introduce helpers to pretty-print buffers.

Also applies to: 1489-1489, 1501-1501, 1513-1513, 1221-1221, 943-944, 2265-2266

---

754-756: Nit: CK_RV values print in decimal; consider hex for readability

CppUnit prints integral types in decimal by default, which is less readable for CKR_* codes. If useful, introduce a small helper (either a custom assertion_traits<CK_RV> or an ASSERT_CKR_EQ macro that formats std::hex) to render CK_RV in hex. Optional, but it makes failures faster to triage.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1a39ad7 and 2c2aa4f.

📒 Files selected for processing (1)

• src/lib/test/ObjectTests.cpp (40 hunks)

🧰 Additional context used

🧬 Code Graph Analysis (1)

src/lib/test/ObjectTests.cpp (1)

src/lib/test/ObjectTests.h (1)

• ObjectTests (41-185)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)

• GitHub Check: macOS (openssl)
• GitHub Check: Windows (x86, openssl)
• GitHub Check: macOS (botan)
• GitHub Check: Linux with OpenSSL 3.0
• GitHub Check: Linux with Botan
• GitHub Check: Linux with OpenSSL 1.1.1

🔇 Additional comments (34)

src/lib/test/ObjectTests.cpp (34)

97-99: Good switch to CPPUNIT_ASSERT_EQUAL(expected, actual) for CK_RV checks

The conversions here correctly use expected-first ordering and will yield clearer diffs for failing return values.

Also applies to: 121-123, 126-128, 152-154, 159-161

---

193-197: Consistent assert conversions in certificate attribute checks

These follow the same expected/actual order and keep the semantics identical while improving diagnostics.

Also applies to: 199-201

---

236-239: LGTM: assert_equal usage in X509 certificate attribute reads

The changes correctly assert CKR_OK for both length and value fetches.

Also applies to: 249-251

---

309-312: LGTM: key attribute checks use CPPUNIT_ASSERT_EQUAL correctly

No behavior change; better failure output for CRYPTOKI calls.

Also applies to: 315-317

---

358-361: LGTM: public key attribute checks converted properly

Expected value is first, return value is second — correct.

Also applies to: 364-366

---

411-414: LGTM: private key attribute checks converted properly

Accurate usage; improves readability of failures.

Also applies to: 417-419

---

451-454: LGTM: attribute mutability tests now show actual CK_RV on failure

Helpful improvement across wrap-with-trusted and sensitive attribute transitions.

Also applies to: 456-458, 460-462, 472-474, 476-478, 480-482

---

496-498: LGTM: RSA public key attribute reads assert CKR_OK

Matches the intended semantics; clearer failure messages.

Also applies to: 502-504

---

533-535: LGTM: RSA private key attribute reads assert CKR_OK

Ordering and intent are correct.

Also applies to: 539-541

---

754-756: LGTM: initialization and first create/destroy checks converted

Expected vs. actual ordering is correct for all CKR_* here.

Also applies to: 762-770

---

785-794: LGTM: read-only + user session assertions converted properly

These changes preserve logic and increase test output clarity.

Also applies to: 795-806

---

815-833: LGTM: read-write session (no login) cases converted

Accurate use of CPPUNIT_ASSERT_EQUAL; no regressions.

Also applies to: 834-841

---

846-865: LGTM: read-write + user session conversions

All CKR checks converted correctly.

Also applies to: 866-877, 879-881

---

886-899: LGTM: read-write + SO session conversions

Expected-first ordering across the block looks good.

Also applies to: 900-917

---

967-969: LGTM: testCopyObject CK_RV assertions converted

Covers the allow/deny flows; expected constants are first, rv second — correct throughout.

Also applies to: 971-979, 991-999, 1003-1011, 1016-1017, 1022-1023, 1025-1027

---

1030-1035: LGTM: copy rules and template consistency conversions

The new assertions will make violations easier to diagnose.

Also applies to: 1037-1046, 1049-1053, 1060-1061, 1063-1071, 1073-1075

---

1103-1113: LGTM: destroy object flow uses CPPUNIT_ASSERT_EQUAL throughout

This significantly improves output when negative paths fail.

Also applies to: 1115-1121, 1123-1129, 1131-1139, 1140-1145, 1147-1157, 1159-1161, 1163-1181, 1183-1189

---

1201-1203: LGTM: get object size path converted correctly

Assertions on CRYPTOKI calls now consistent with the rest of the file.

Also applies to: 1205-1211, 1213-1220, 1223-1225

---

1238-1244: LGTM: get attribute value path converted correctly

Argument validation and happy-path checks are clearer now.

Also applies to: 1246-1252, 1254-1268, 1270-1276

---

1338-1344: LGTM: set attribute value path converted

The read-only and action-prohibited flows now show actual CK_RV on failure.

Also applies to: 1346-1352, 1354-1368, 1375-1389, 1392-1400, 1403-1410

---

1426-1432: LGTM: find objects flow converted

Great coverage of logged-in and logged-out scenarios; the new asserts are consistent.

Also applies to: 1434-1440, 1442-1468, 1470-1477, 1481-1482, 1485-1489, 1493-1501, 1504-1514

---

1527-1533: LGTM: key generation flow converted

All CKR paths use the improved assertion macro.

Also applies to: 1535-1541, 1543-1558, 1560-1567, 1569-1585

---

1596-1602: LGTM: certificate creation flow converted

Template incomplete and read-only attribute update cases clearly reported now.

Also applies to: 1604-1613, 1619-1621

---

1639-1645: LGTM: default data attributes setup converted

Initialization and create checks now consistent with the rest.

Also applies to: 1647-1653, 1655-1658

---

1684-1690: LGTM: default X509 cert attributes setup converted

Accurate expected-first ordering; no logic change.

Also applies to: 1692-1698, 1701-1705

---

1736-1742: LGTM: default RSA public attributes setup converted

All relevant CKR paths updated.

Also applies to: 1744-1750, 1753-1758

---

1799-1805: LGTM: default RSA private attributes setup converted

Consistent use of CPPUNIT_ASSERT_EQUAL.

Also applies to: 1807-1814, 1816-1823

---

1854-1860: LGTM: always/never attribute tests converted

These will be easier to debug if a token deviates from the spec.

Also applies to: 1862-1868, 1870-1878, 1882-1884, 1886-1890

---

1924-1930: LGTM: sensitive attribute tests converted

Clearer negative and positive path reporting.

Also applies to: 1932-1938, 1942-1944, 1949-1956

---

1979-1985: LGTM: invalid attribute access converted

Accurate expected CKR values asserted.

Also applies to: 1987-1993, 1995-1997

---

2034-2040: LGTM: re-authentication flows converted

These cover a lot of nuanced states; the improved assertions are beneficial.

Also applies to: 2042-2048, 2050-2068, 2070-2078, 2080-2086, 2088-2102, 2105-2116, 2119-2125

---

2136-2142: LGTM: allowed mechanisms tests converted

Using CPPUNIT_ASSERT_EQUAL for CKR values improves test clarity.

Also applies to: 2144-2156, 2159-2168, 2171-2194

---

2234-2240: LGTM: template attribute tests converted

Initialization and attribute fetch sequences now consistently assert CKR results.

Also applies to: 2242-2248, 2257-2260, 2263-2266, 2288-2290

---

2322-2328: LGTM: secret key creation and KCV checks converted

Return code assertions across all key types are correctly switched.

Also applies to: 2330-2336, 2379-2387, 2391-2399, 2403-2411, 2415-2423, 2427-2435

[bukka]

Great, although I think it reads better with the arguments swapped, "assert that this variable under test rv equals this golden reference constant CKR_OK"

This is done because of the reporting in the failure where it prints expected and the actual value. It expects the first value to be the expected as it's clear from the definition:

#define CPPUNIT_ASSERT_EQUAL(expected,actual)          \
  ( CPPUNIT_NS::assertEquals( (expected),              \
                              (actual),                \
                              CPPUNIT_SOURCELINE(),    \
                              "" ) )

[kalvdans]

Great, although I think it reads better with the arguments swapped, "assert that this variable under test rv equals this golden reference constant CKR_OK"

This is done because of the reporting in the failure where it prints expected and the actual value. It expects the first value to be the expected as it's clear from the definition:

Aha! Then the correct order, it is. Greetings from Yoda. ;)