Skip to content

Test change, check size of cache before calling the second server#2526

Merged
jwillemsen merged 1 commit intoDOCGroup:masterfrom
jwillemsen:jwi-idletestimpro
Apr 8, 2026
Merged

Test change, check size of cache before calling the second server#2526
jwillemsen merged 1 commit intoDOCGroup:masterfrom
jwillemsen:jwi-idletestimpro

Conversation

@jwillemsen
Copy link
Copy Markdown
Member

@jwillemsen jwillemsen commented Apr 8, 2026

* TAO/tests/Transport_Idle_Timeout/Echo_i.cpp:
* TAO/tests/Transport_Idle_Timeout/client_multiple.cpp:

Summary by CodeRabbit

  • Tests
    • Simplified control flow for cache-size verification in idle timeout test scenarios
    • Updated test parameters to align with revised verification logic

    * TAO/tests/Transport_Idle_Timeout/Echo_i.cpp:
    * TAO/tests/Transport_Idle_Timeout/client_multiple.cpp:
@jwillemsen
Copy link
Copy Markdown
Member Author

Improved test from #2523

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 8, 2026

Walkthrough

The changes refactor test code to simplify control flow in Echo_i::ping by removing a conditional wrapper around a cache-size verification check, and adjust numeric parameters in client ping invocations across Transport_Idle_Timeout test cases.

Changes

Cohort / File(s) Summary
Echo_i Control Flow Simplification
TAO/tests/Transport_Idle_Timeout/Echo_i.cpp
Removed else block wrapping the cache-size check for "ping"; the check now runs unconditionally immediately after sleep_with_reactor(), while the conditional "ping_second" check remains guarded by the server nil-check.
Client Ping Parameter Adjustment
TAO/tests/Transport_Idle_Timeout/client_multiple.cpp
Changed second argument in CORBA ping invocations from 2 to 1 across multiple test case invocations (TC-1 initial and post-timeout calls, TC-2 reconnect verification).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

Possibly related PRs

Poem

🐰 A rabbit hops through test code with glee,
Control flow flows now, simplified and free!
Parameters tweak from two down to one,
Transport tests hop—refactoring's done! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: moving the cache-size verification check to always run before the second server interaction, which is the primary modification across both files.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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

@jwillemsen jwillemsen changed the title Tets change, check size of cache before calling the second server Test change, check size of cache before calling the second server Apr 8, 2026
@jwillemsen jwillemsen merged commit 80991c7 into DOCGroup:master Apr 8, 2026
38 checks passed
@jwillemsen jwillemsen deleted the jwi-idletestimpro branch April 8, 2026 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant