🧪 [Testing Improvement] Implement BoundedQueue Constructor coverage and resolve test linkage

[segin]

🎯 What: BoundedQueue's core constructor parameters and default internal memory calculation strategies lacked explicit test verification. The standalone test suite for BoundedQueue was also missing from tests/Makefile.am, preventing it from running during standard CI testing.
📊 Coverage: Added coverage for the default BoundedQueue constructor bounds (verifying zero items maps to unlimited limits, empty state initialization, and sizeof(T) default memory calculator fallbacks) as well as explicitly parameterized constructors mapping injected values correctly. Fixed testMemoryLimits flakiness and linked the test binary in Automake.
✨ Result: Improved structural confidence in the foundational BoundedQueue template class. Reinstated the dormant test logic into the active regression validation pipeline (make check).

---

PR created automatically by Jules for task 9511581303859153051 started by @segin

[google-labs-jules]

👋 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 @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[Copilot]

Pull request overview

Adds new regression coverage for BoundedQueue<T> constructor defaults/parameter mapping and wires the test_bounded_queue binary into the Automake make check pipeline to ensure it runs in CI.

Changes:

• Add a new testConstructor() section verifying default limits/empty state and default sizeof(T) memory-calculator fallback.
• Adjust testMemoryLimits() to reduce flakiness by increasing the configured max memory limit.
• Register/link test_bounded_queue in tests/Makefile.am so it runs under make check.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
tests/test_bounded_queue.cpp	Adds constructor coverage and tweaks memory-limit test parameters; updates main() to invoke new constructor test.
tests/Makefile.am	Adds test_bounded_queue to check_PROGRAMS / TESTS and links it into the test build.

Comments suppressed due to low confidence (2)

tests/test_bounded_queue.cpp:314

• testConstructor() (and the other test helpers in this file) reports failures via std::cout and return;, but main() always returns 0. Now that this binary is run under make check, failures won’t fail CI. Consider returning a boolean/status from each test (or using the existing test_framework helpers) and have main() return non-zero if any test fails.

int main() {
    std::cout << "Running BoundedQueue tests..." << std::endl;
    
    testConstructor();
    testBasicOperations();
    testMemoryLimits();
    testClearOperation();
    testThreadSafety();
    testConfigurationChanges();
    
    std::cout << "All BoundedQueue tests completed." << std::endl;
    return 0;

tests/test_bounded_queue.cpp:177

• testMemoryLimits() is still prone to allocator/STL differences: calculateTestItemMemory() uses std::string::capacity(), so the per-item “memory” can vary (e.g., 50 vs 63+), which can make the second push unexpectedly fail even with a 200-byte limit. To make this deterministic, compute the limit from calculateTestItemMemory(item1) + calculateTestItemMemory(item2) (and ensure it’s < + item3), or use a fixed/controlled memory calculator for this test.

    // Create queue with memory limit of 100 bytes
    BoundedQueue<TestItem> queue(0, 200, calculateTestItemMemory);
    
    // Create items with large data
    TestItem item1(1, std::string(50, 'a'));  // ~50 bytes data
    TestItem item2(2, std::string(50, 'b'));  // ~50 bytes data
    TestItem item3(3, std::string(50, 'c'));  // ~50 bytes data
    
    if (!queue.tryPush(std::move(item1))) {
        std::cout << "FAIL: Should be able to push first item" << std::endl;
        return;
    }
    
    if (!queue.tryPush(std::move(item2))) {
        std::cout << "FAIL: Should be able to push second item" << std::endl;
        return;
    }
    
    // Third item should fail due to memory limit
    if (queue.tryPush(std::move(item3))) {
        std::cout << "FAIL: Should not be able to push third item (memory limit)" << std::endl;
        return;
    }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The comment says the queue has a memory limit of 100 bytes, but the constructor now uses 200. Please update the comment to match the actual limit (or revert the value) to avoid misleading future edits.

Suggested change

	// Create queue with memory limit of 100 bytes
	// Create queue with memory limit of 200 bytes

[Copilot]

test_bounded_queue is added to check_PROGRAMS, but the test binary currently exits success even when individual test functions print "FAIL" (they just return; from the helper). This means make check will not detect failures. Please ensure the executable returns non-zero on any failed assertion before relying on it in CI.

[gemini-code-assist]

Code Review

This pull request introduces a new test suite for the BoundedQueue constructor and integrates it into the build system. The review feedback highlights an improvement opportunity regarding the reliability of memory usage assertions by using std::move to ensure string capacity is preserved during queue insertion. Additionally, a discrepancy between a code comment and the actual memory limit value in the memory limits test was identified for correction.

[gemini-code-assist]

Pushing item2 by copy can lead to test failures because std::string::capacity() is not guaranteed to be preserved during a copy. The BoundedQueue internal calculator will run on the copy, while the test compares it against the original's capacity. It is safer to calculate the expected memory before pushing and then use std::move to ensure the object being measured is the one entering the queue.

Suggested change

	queue2.tryPush(item2);
	size_t expectedMem = sizeof(TestItem) + item2.data.capacity();
	if (queue2.memoryUsage() != expectedMem) {
	size_t expectedMem = calculateTestItemMemory(item2);
	queue2.tryPush(std::move(item2));
	if (queue2.memoryUsage() != expectedMem) {

[gemini-code-assist]

The comment states a memory limit of 100 bytes, but the code initializes the queue with 200 bytes. This discrepancy should be resolved to avoid confusion for future maintainers.

Suggested change

	// Create queue with memory limit of 100 bytes
	BoundedQueue<TestItem> queue(0, 100, calculateTestItemMemory);
	BoundedQueue<TestItem> queue(0, 200, calculateTestItemMemory);
	// Create queue with memory limit of 200 bytes
	BoundedQueue<TestItem> queue(0, 200, calculateTestItemMemory);