Skip to content

Add some unit tests - #782

Merged
knoepfel merged 5 commits into
Framework-R-D:mainfrom
marcpaterno:add-some-unit-tests
Jul 31, 2026
Merged

Add some unit tests#782
knoepfel merged 5 commits into
Framework-R-D:mainfrom
marcpaterno:add-some-unit-tests

Conversation

@marcpaterno

@marcpaterno marcpaterno commented Jul 28, 2026

Copy link
Copy Markdown
Member

This adds two new tests.
One is a unit test for the transform_node.

The other is a test (it is not clear to me that it is a unit test) for the infrastructure that does the join for a the kind of transform_node that combines two products from different data layers.

  • Tests

    • Added transform_node unit tests for single-output transforms and multi-output product storage.
    • Verified output metadata, products, message IDs, store lineage, and call/product counters.
    • Added multilayer_join_node<2> coverage for joining products from separate data layers.
    • Verified that output waits for both index messages and that joined tuples preserve source indexes.
    • Flushed token ports to complete graph processing without warnings.
  • Build system

    • Registered transform_node_test.cpp and multilayer_join_test.cpp in test/CMakeLists.txt.
    • Linked both tests with Catch2 and phlex::core_internal.
  • Code quality

    • Applied clang-format to the test changes.

@marcpaterno
marcpaterno requested a review from knoepfel July 28, 2026 19:59
@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: a5f4d904-0b1c-430d-a8bd-f76a69b75690

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Added Catch2 tests for transform_node and multilayer_join_node<2>. Registered both tests in CMake with phlex::core_internal.

Changes

Node tests

Layer / File(s) Summary
Transform node coverage
test/transform_node_test.cpp, test/CMakeLists.txt
Tests single-output and multi-output transformations, including products, metadata, store lineage, algorithm source, messages, and counters.
Multilayer join coverage
test/multilayer_join_test.cpp, test/CMakeLists.txt
Tests staged delivery from two inputs, output suppression until both indexes arrive, joined contents, source indexes, and token flushing.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested reviewers: knoepfel

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title is related to the changes, but it is too generic and does not identify the tested components or join coverage. Use a specific title such as "Add transform_node and multilayer_join_node unit tests".
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
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.
✨ 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.

@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

@@            Coverage Diff             @@
##             main     #782      +/-   ##
==========================================
- Coverage   83.90%   83.84%   -0.06%     
==========================================
  Files         172      172              
  Lines        7231     7231              
  Branches      850      850              
==========================================
- Hits         6067     6063       -4     
- Misses        883      885       +2     
- Partials      281      283       +2     
Flag Coverage Δ
scripts 80.09% <ø> (ø)
unittests 85.72% <ø> (-0.09%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 3 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d352f0f...54a0ee5. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Move the multi-input join coverage from `transform_node` tests to a
dedicated `multilayer_join_node` test by renaming `join_test.cpp` to
`multilayer_join_test.cpp`, updating the test case name, and adjusting
the test wiring. Remove the old join test from
`test/transform_node_test.cpp` and update `test/CMakeLists.txt`.
@marcpaterno

Copy link
Copy Markdown
Member Author

@phlexbot format

@github-actions

Copy link
Copy Markdown
Contributor

Format Fixes Applied

✅ clang-format fixes pushed (commit ffcf083)

⚠️ Note: Some issues may require manual review.

@marcpaterno
marcpaterno marked this pull request as ready for review July 31, 2026 16:13

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@test/multilayer_join_test.cpp`:
- Around line 35-39: Remove the unused selector template helper from the test
fixture, including its product_selector construction logic, since it has no call
sites in the file.
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 613f70a4-b1d0-44f2-8744-9d76a4340de4

📥 Commits

Reviewing files that changed from the base of the PR and between d352f0f and ffcf083.

📒 Files selected for processing (3)
  • test/CMakeLists.txt
  • test/multilayer_join_test.cpp
  • test/transform_node_test.cpp
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
  • GitHub Check: Analyze actions with CodeQL
  • GitHub Check: Analyze cpp with CodeQL
  • GitHub Check: build (gcc, none)
  • GitHub Check: coverage
  • GitHub Check: clang-tidy-check
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{cpp,cc,cxx,h,hpp}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{cpp,cc,cxx,h,hpp}: Use clang-format tool for all C++ code formatting (VS Code auto-formats on save); configuration defined in .clang-format with 100-character line limit and 2-space indentation
Follow clang-tidy recommendations defined in .clang-tidy

Files:

  • test/transform_node_test.cpp
  • test/multilayer_join_test.cpp
**/*.{hpp,cpp}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{hpp,cpp}: Use .hpp for header files, .cpp for implementation, and *_test.cpp for test files in C++
Enforce 100-character line limit and 2-space indentation in C++ code via .clang-format
Use QualifierAlignment: Right (east-const) style: int const x not const int x in C++
Use PointerAlignment: Left in C++ (pointer * attached to type, not variable name)
All C++ identifiers must use lower_case naming: namespaces, classes, structs, enums, functions, variables, parameters, members, and constants
Exception to C++ naming: template parameters use CamelCase
Exception to C++ naming: macros use UPPER_CASE
Private, protected, and constant members in C++ must have a trailing underscore (_), no trailing underscore on anything else
Use enum class preferred over plain enum in C++
Use std::shared_ptr for shared ownership, std::unique_ptr for exclusive ownership, raw pointers for non-owning references only in C++
Use functors with agent-noun pattern: ModelEvaluator evaluate_model(...) in C++
Apply .clang-tidy checks for bugprone, cert, clang-analyzer, concurrency, cppcoreguidelines, misc, modernize, performance, portability, and readability as defined in the .clang-tidy configuration file
Use phlex:: namespace for core code, phlex::experimental:: for experimental features in C++

Files:

  • test/transform_node_test.cpp
  • test/multilayer_join_test.cpp
🪛 Cppcheck (2.21.0)
test/multilayer_join_test.cpp

[style] 36-36: The function 'selector' is never used.

(unusedFunction)

🔇 Additional comments (3)
test/transform_node_test.cpp (1)

1-153: LGTM!

test/CMakeLists.txt (1)

212-227: LGTM!

test/multilayer_join_test.cpp (1)

1-34: LGTM!

Also applies to: 41-105

Comment thread test/multilayer_join_test.cpp Outdated
@knoepfel
knoepfel merged commit 0036e91 into Framework-R-D:main Jul 31, 2026
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants