🧪 [Test Improvement] Implement test coverage for MemoryOptimizer Core#271
🧪 [Test Improvement] Implement test coverage for MemoryOptimizer Core#271
Conversation
Co-authored-by: segin <480709+segin@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
There was a problem hiding this comment.
Pull request overview
This PR aims to increase automated coverage around the I/O memory optimization stack by adding a core test for MemoryOptimizer behavior (pool trimming / limit enforcement) and wiring the relevant test binaries into the Automake test runner.
Changes:
- Added a new
testMemoryOptimizerCore()flow totests/test_memory_optimizer.cppthat populatesIOBufferPool, invokesMemoryOptimizer::optimizeMemoryUsage(), and asserts trimming occurred. - Simplified some existing test output in
tests/test_memory_optimizer.cpp(BoundedQueue stats printing; MemoryTracker pressure printing). - Integrated
test_memory_optimizerandtest_memory_leak_preventionintotests/Makefile.amso they are built and executed viamake check.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
tests/test_memory_optimizer.cpp |
Adds a new “core” memory optimizer test and updates existing test output. |
tests/Makefile.am |
Registers new test executables and includes them in check_PROGRAMS (and therefore TESTS). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Set a critical memory pressure level | ||
| // Memory usage is 716800 bytes, total limit is 1MB. (716KB / 1024KB = 70%) | ||
| // Let's force optimization manually or change memory limits to trigger pressure | ||
| optimizer.setMemoryLimits(700 * 1024, 512 * 1024); | ||
|
|
||
| // Call optimizeMemoryUsage | ||
| optimizer.optimizeMemoryUsage(); | ||
|
|
||
| std::cout << "Pool size after optimization: " << pool.getStats().at("current_pool_size") << " bytes" << std::endl; | ||
|
|
||
| // Assert cache trimming | ||
| if (pool.getStats().at("current_pool_size") >= 65536) { | ||
| std::cerr << "Assertion failed: Pool size was not trimmed during optimization!" << std::endl; | ||
| std::exit(1); | ||
| } |
There was a problem hiding this comment.
testMemoryOptimizerCore() assumes lowering setMemoryLimits() and calling optimizeMemoryUsage() will immediately perform aggressive trimming and drop current_pool_size below 65536. In MemoryOptimizer, the optimization behavior depends on the cached m_memory_pressure_level, which is only recomputed by the monitoring thread (every 5s), so this test is timing-dependent and will often run with Normal pressure (where the pool limit becomes m_max_total_memory/4, i.e. ~175KB here) and fail. Make the assertion deterministic by either (a) waiting/polling until optimizer.getMemoryPressureLevel() reaches the expected level before optimizing, or (b) asserting current_pool_size is <= the expected max derived from the current pressure level and m_max_total_memory, instead of a hard-coded 64KB threshold.
| check_PROGRAMS += test_codec_performance test_codec_thread_safety test_codec_concurrent_instances test_codec_performance_simple test_codec_thread_safety_simple test_threading_safety_baseline test_audio_thread_safety test_audio_threading_pattern test_iohandler_thread_safety_comprehensive test_iohandler_memory_deadlock_prevention test_memory_pool_manager_integration test_memory_pool_manager_thread_safety_comprehensive test_memory_pool_manager_basic_threading test_memory_pool_allocation_failure test_surface_thread_safety test_surface_performance_regression test_system_wide_threading_integration test_threading_performance_regression test_memory_optimizer test_memory_leak_prevention | ||
|
|
There was a problem hiding this comment.
Adding test_memory_leak_prevention to check_PROGRAMS means it will run under make check (since TESTS = $(check_PROGRAMS) in this Makefile). tests/test_memory_leak_prevention.cpp performs real HTTP requests (e.g., to httpbin.org), which makes the test suite dependent on external network availability and can be flaky/fail in CI or offline builds. Consider keeping it as a manual/debug test (buildable but not in TESTS), or refactor it to avoid external network (mock server/fixture, or gate execution behind an env var/feature check).
| check_PROGRAMS += test_codec_performance test_codec_thread_safety test_codec_concurrent_instances test_codec_performance_simple test_codec_thread_safety_simple test_threading_safety_baseline test_audio_thread_safety test_audio_threading_pattern test_iohandler_thread_safety_comprehensive test_iohandler_memory_deadlock_prevention test_memory_pool_manager_integration test_memory_pool_manager_thread_safety_comprehensive test_memory_pool_manager_basic_threading test_memory_pool_allocation_failure test_surface_thread_safety test_surface_performance_regression test_system_wide_threading_integration test_threading_performance_regression test_memory_optimizer test_memory_leak_prevention | |
| check_PROGRAMS += test_codec_performance test_codec_thread_safety test_codec_concurrent_instances test_codec_performance_simple test_codec_thread_safety_simple test_threading_safety_baseline test_audio_thread_safety test_audio_threading_pattern test_iohandler_thread_safety_comprehensive test_iohandler_memory_deadlock_prevention test_memory_pool_manager_integration test_memory_pool_manager_thread_safety_comprehensive test_memory_pool_manager_basic_threading test_memory_pool_allocation_failure test_surface_thread_safety test_surface_performance_regression test_system_wide_threading_integration test_threading_performance_regression test_memory_optimizer | |
| # Network-dependent manual/debug test; keep buildable but do not run under `make check` | |
| noinst_PROGRAMS += test_memory_leak_prevention |
🎯 What: Improved test coverage for
src/io/MemoryOptimizer.cppwhich was lacking direct assertions for cache trimming and memory limit enforcement.📊 Coverage: Added
testMemoryOptimizerCoreintests/test_memory_optimizer.cppwhich asserts thatMemoryOptimizereffectively trims the memory allocated byIOBufferPoolunder artificial memory pressure viaoptimizeMemoryUsage(). Also integratedtest_memory_leak_preventioncleanly intotests/Makefile.am.✨ Result: Improved robustness by dynamically testing the memory optimizer bounds behavior under simulated component utilization.
PR created automatically by Jules for task 10493883677827715327 started by @segin