Add py_unittest_qnx_test.bzl macro for QNX test suites

[saksham-chawla]

This PR introduces a new convenience macro py_unittest_qnx_test in third_party/itf to simplify the creation of QNX unit test suites across the repository.

---

Add py_unittest_qnx_test.bzl macro for QNX test suites

This PR introduces a new convenience macro py_unittest_qnx_test in third_party/itf to simplify the creation of QNX unit test suites across the repository. It also updates the existing cc_unit_test_suites_for_host_and_qnx macro in bazel/unit_tests.bzl to leverage this new macro, allowing for a more streamlined definition of QNX tests.

Architecture Diagram:

flowchart TD
    subgraph Bazel_Macros
        A["third_party/itf/py_unittest_qnx_test.bzl (+45 lines)"] -- defines --> B(py_unittest_qnx_test macro)
        C["bazel/unit_tests.bzl (+3 lines)"] -- loads --> B
        C -- defines --> D(cc_unit_test_suites_for_host_and_qnx macro)
        D -- calls --> B
    end

Loading

---

This summary was automatically generated by @propel-code-bot

[propel-code-bot]

Action required: critical logic omissions drop kwargs and exclusions in QNX tests.

Status: Action Required | Risk: High | Readiness: 3/10 (Needs Significant Work)

Issues Identified & Suggestions

• Forward **kwargs to preserve testonly flags in test_suite: third_party/itf/py_unittest_qnx_test.bzl
• Propagate excluded_tests_filter to QNX wrappers to honor exclusions: third_party/itf/py_unittest_qnx_test.bzl

Review Details

📁 2 files reviewed | 💬 2 comments

👍 / 👎 individual comments to help improve reviews for you

[propel-code-bot]

[Logic] **kwargs is declared in the function signature and documented as "Additional arguments passed to test_suite", but it is never forwarded to native.test_suite(). The caller in bazel/unit_tests.bzl passes testonly = True (line 56 of that file), which is not an explicit parameter of py_unittest_qnx_test, so it lands in **kwargs and is silently dropped. The resulting test_suite will not be marked testonly = True, which can cause Bazel build errors when non-testonly targets depend on testonly targets.

The native.test_suite(...) call should unpack **kwargs, e.g.:

native.test_suite(
    name = name,
    tests = qnx_test_targets + test_suites,
    visibility = visibility,
    tags = tags,
    **kwargs
)

Context for Agents

[**Bug**] `**kwargs` is declared in the function signature and documented as "Additional arguments passed to test_suite", but it is never forwarded to `native.test_suite()`. The caller in `bazel/unit_tests.bzl` passes `testonly = True` (line 56 of that file), which is not an explicit parameter of `py_unittest_qnx_test`, so it lands in `**kwargs` and is silently dropped. The resulting `test_suite` will not be marked `testonly = True`, which can cause Bazel build errors when non-testonly targets depend on testonly targets.

The `native.test_suite(...)` call should unpack `**kwargs`, e.g.:
```python
native.test_suite(
    name = name,
    tests = qnx_test_targets + test_suites,
    visibility = visibility,
    tags = tags,
    **kwargs
)
```

File: third_party/itf/py_unittest_qnx_test.bzl
Line: 57

Chain of Thought

• Validation Decision: accepted
• Confidence: 0.97
• Likelihood Score: 5/5
• Likelihood Reason: The caller in bazel/unit_tests.bzl explicitly passes testonly=True which lands in **kwargs and is silently dropped. This will inevitably cause the test_suite to not be marked testonly, which can lead to Bazel build errors.
• Suggested Action: keep
• Tool Calls Used: 2

Reasoning

The comment accurately identifies a bug introduced by this PR:

1. **kwargs accepted but not forwarded: The function py_unittest_qnx_test accepts **kwargs (line 24) and documents it as "Additional arguments passed to test_suite" (line 36), but the native.test_suite() call at lines 52-57 does not include **kwargs.

2. Concrete caller passes testonly = True: In bazel/unit_tests.bzl at lines 54-60, the function is called with testonly = True. Since testonly is not an explicit parameter of py_unittest_qnx_test, it goes into **kwargs and is silently discarded.

3. This is a new issue: The base version of the file was just a stub (def py_unittest_qnx_test(**kwargs): pass), so this bug is entirely introduced by this PR.

4. The fix is concrete and actionable: Adding **kwargs to the native.test_suite() call is the correct fix, as shown in the suggested code.

5. Real impact: The testonly attribute is important in Bazel - if a test_suite is not marked testonly but depends on testonly targets, Bazel will emit errors. This is not a theoretical concern; it will manifest immediately when the code is used as intended.

Evidence Summary

1. File Snapshot (HEAD)

• File: bazel/unit_tests.bzl

1: # *******************************************************************************
2: # Copyright (c) 2025 Contributors to the Eclipse Foundation
3: #
4: # See the NOTICE file(s) distributed with this work for additional
5: # information regarding copyright ownership.
6: #
7: # This program and the accompanying materials are made available under the
8: # terms of the Apache License Version 2.0 which is available at
9: # https://www.apache.org/licenses/LICENSE-2.0
10: #
...

2. File Snapshot (HEAD)

• File: third_party/itf/py_unittest_qnx_test.bzl

1: # *******************************************************************************
2: # Copyright (c) 2025 Contributors to the Eclipse Foundation
3: #
4: # See the NOTICE file(s) distributed with this work for additional
5: # information regarding copyright ownership.
6: #
7: # This program and the accompanying materials are made available under the
8: # terms of the Apache License Version 2.0 which is available at
9: # https://www.apache.org/licenses/LICENSE-2.0
10: #
...

[propel-code-bot]

[Logic] excluded_tests_filter is accepted but never consumed. cc_unit_test_suites_for_host_and_qnx still forwards the caller-provided patterns (see the docstring describing FooTest.Test1, etc.), yet this macro drops them, so tests that users explicitly excluded now execute on QNX again. Forward the filter when creating each QNX wrapper (or otherwise remove the matching tests) so the documented behavior is preserved.

One straightforward fix is to pass the filter to the project-specific wrapper so it can apply the appropriate gtest filter:

            cc_test_qnx(
                name = qnx_target_name,
                cc_test = test_case,
                excluded_tests_filter = excluded_tests_filter,
            )

Context for Agents

`excluded_tests_filter` is accepted but never consumed. `cc_unit_test_suites_for_host_and_qnx` still forwards the caller-provided patterns (see the docstring describing `FooTest.Test1`, etc.), yet this macro drops them, so tests that users explicitly excluded now execute on QNX again. Forward the filter when creating each QNX wrapper (or otherwise remove the matching tests) so the documented behavior is preserved.

One straightforward fix is to pass the filter to the project-specific wrapper so it can apply the appropriate gtest filter:

```python
            cc_test_qnx(
                name = qnx_target_name,
                cc_test = test_case,
                excluded_tests_filter = excluded_tests_filter,
            )
```

File: third_party/itf/py_unittest_qnx_test.bzl
Line: 22

Chain of Thought

• Validation Decision: accepted
• Confidence: 0.92
• Likelihood Score: 4/5
• Likelihood Reason: The excluded_tests_filter parameter is accepted but demonstrably unused in the function body. Any caller passing an exclusion filter (as cc_unit_test_suites_for_host_and_qnx does) will have their excluded tests silently re-included in QNX test runs. This is a real functional regression introduced by this PR (previously the macro was a no-op, so no tests ran at all).
• Suggested Action: keep
• Tool Calls Used: 3

Reasoning

The comment accurately identifies a real bug introduced by this PR:

1. Parameter accepted but unused: excluded_tests_filter is declared as a parameter on line 22 of py_unittest_qnx_test, but it is never referenced in the function body. The cc_test_qnx() call on lines 45-48 only passes name and cc_test.

2. Caller passes the filter: In bazel/unit_tests.bzl line 58, cc_unit_test_suites_for_host_and_qnx passes excluded_tests_filter = excluded_tests_filter to py_unittest_qnx_test, expecting it to be applied.

3. This is a new issue from this PR: In the BASE version, py_unittest_qnx_test was a no-op stub (def py_unittest_qnx_test(**kwargs): pass), so no QNX tests ran at all. Now that this PR gives it a real implementation that creates QNX test wrappers, the exclusion filter being dropped becomes a functional bug — tests that users intended to exclude will now run on QNX.

4. The suggested fix is concrete and actionable: The comment suggests forwarding excluded_tests_filter to cc_test_qnx(), with a specific code example.

5. Missing documentation: The excluded_tests_filter parameter is also not documented in the py_unittest_qnx_test docstring Args section, further suggesting it was overlooked during implementation.

Evidence Summary

1. File Snapshot (HEAD)

• File: third_party/itf/py_unittest_qnx_test.bzl

1: # *******************************************************************************
2: # Copyright (c) 2025 Contributors to the Eclipse Foundation
3: #
4: # See the NOTICE file(s) distributed with this work for additional
5: # information regarding copyright ownership.
6: #
7: # This program and the accompanying materials are made available under the
8: # terms of the Apache License Version 2.0 which is available at
9: # https://www.apache.org/licenses/LICENSE-2.0
10: #
...

2. File Snapshot (HEAD)

• File: bazel/unit_tests.bzl

1: # *******************************************************************************
2: # Copyright (c) 2025 Contributors to the Eclipse Foundation
3: #
4: # See the NOTICE file(s) distributed with this work for additional
5: # information regarding copyright ownership.
6: #
7: # This program and the accompanying materials are made available under the
8: # terms of the Apache License Version 2.0 which is available at
9: # https://www.apache.org/licenses/LICENSE-2.0
10: #
...

3. File Snapshot (BASE)

• File: bazel/unit_tests.bzl

1: # *******************************************************************************
2: # Copyright (c) 2025 Contributors to the Eclipse Foundation
3: #
4: # See the NOTICE file(s) distributed with this work for additional
5: # information regarding copyright ownership.
6: #
7: # This program and the accompanying materials are made available under the
8: # terms of the Apache License Version 2.0 which is available at
9: # https://www.apache.org/licenses/LICENSE-2.0
10: #
...