Minor cleanups in test_reduction.cpp

[wujingyue]

No description provided.

[github-actions]

Auto-merge Status

✅ Internal CI is finished
❌ No failed checks (nvfuser-ci/jit_python_distributed_tests_20_H100)
✅ PR is mergeable
ℹ️ PR mergeable_state: unstable

Description

• Modernize test parameter handling using structured bindings instead of std::get

• Clean up testing namespace usage by removing :: prefixes

• Improve code readability with consistent formatting

• Update lambda parameter destructuring for better maintainability

Changes walkthrough

Relevant files

Enhancement

test_reduction.cpp Modernize test parameter handling and cleanup                        tests/cpp/test_reduction.cpp Replace std::get(info.param) with structured bindings auto [var1, var2] = info.param Remove :: prefix from ::testing:: to use testing:: consistently Add blank line after TEST_P function for better formatting Update lambda parameter destructuring in INSTANTIATE_TEST_SUITE_P calls +11/-12

PR Reviewer Guide

Here are some key observations to aid the review process:

🧪 PR contains tests

⚡ No major issues detected

Test failures

• (High, 46) NCCL NVLink-SHARP (NVLS) multicast binding failures across multidevice test suites (dlcluster_viking_ci)

  Test Name	H100 (dist.)	Source
  tests.python.multidevice.test_communication.test_allgather	❌
  tests.python.multidevice.test_communication.test_allgather_expanded_broadcast	❌
  tests.python.multidevice.test_communication.test_allreduce	❌
  tests.python.multidevice.test_communication.test_reduce_scatter	❌
  tests.python.multidevice.test_communication.test_reduce_scatter_noncontiguous	❌
  tests.python.multidevice.test_dtensor.test_column_parallel_linear	❌
  tests.python.multidevice.test_dtensor.test_plus_one	❌
  tests.python.multidevice.test_dtensor.test_row_parallel_linear	❌
  tests.python.multidevice.test_expert_parallel.test_dispatch_and_combine	❌
  tests.python.multidevice.test_matmul.test_column_parallel_grouped_mm	❌
  ... with 36 more test failures omitted. Check internal logs.

• (High, 1) NCCL invalid usage error in multidevice overlap test (test_overlap_allgather_matmul_shard_outermost)

  Test Name	H100 (dist.)	Source
  tests.python.multidevice.test_overlap.test_overlap_allgather_matmul_shard_outermost[backend_type=CommunicatorBackend.cuda]	❌

[wujingyue]

The extra pair of parenthesis is the trick to allow structured bindings. I'm not sure about the reason -- it has something to do with C++ templates.

[wujingyue]

!test

[greptile-apps]

Greptile Overview

Greptile Summary

This PR performs minor cleanups to test instantiation macros in test_reduction.cpp:

1. Namespace consistency: Removes ::testing:: prefix in favor of testing:: to match the rest of the file
2. Modern C++ syntax: Replaces std::get<N>(info.param) with structured bindings auto [var1, var2] = info.param in test name generator lambdas
3. Required lambda parentheses: Adds extra parentheses around lambdas ([](const testing::TestParamInfo<T>& info) {...}) which are required when using structured bindings with TestParamInfo (see googletest issue #3848)
4. Formatting: Adds blank line after test function for better readability

These changes improve code consistency and leverage C++20 features (project uses C++20 standard). The structured binding syntax is cleaner and more readable than explicit std::get<> calls. All changes follow patterns already established in other test files like test_welford.cpp and test_multidevice_sharding.cpp.

Confidence Score: 5/5

• This PR is safe to merge with minimal risk - it contains only stylistic improvements to test code
• Score reflects that these are minor, well-tested cleanups following established patterns. Changes: (1) namespace prefix removal is purely stylistic and maintains consistency, (2) structured bindings are a C++17 feature fully supported by the project's C++20 standard, (3) lambda parentheses are required by googletest when using structured bindings (documented workaround for issue Enable resize scheduler by default #3848), (4) formatting change is benign. All changes follow patterns already used in test_welford.cpp and test_multidevice_sharding.cpp. No functional logic changes, only improved readability.
• No files require special attention

Important Files Changed

File Analysis

Filename	Score	Overview
tests/cpp/test_reduction.cpp	5/5	Minor cleanups: removed ::testing:: prefix, modernized with structured bindings, added required lambda parentheses

Sequence Diagram

sequenceDiagram
    participant Dev as Developer
    participant Test as Test Framework
    participant GTest as GoogleTest
    participant Code as Test Code
    
    Note over Dev,Code: PR Changes: Modernize Test Instantiation
    
    Dev->>Code: Remove ::testing:: prefix
    Note right of Code: testing::Combine instead<br/>of ::testing::Combine
    
    Dev->>Code: Replace std::get with structured bindings
    Note right of Code: auto [ndims, inner_size] = info.param<br/>instead of std::get<0>(info.param)
    
    Dev->>Code: Add lambda parentheses
    Note right of Code: ([](const testing::TestParamInfo...) {...})<br/>Required for structured bindings
    
    Test->>GTest: INSTANTIATE_TEST_SUITE_P
    GTest->>Code: Call name generator lambda
    Code->>Code: Unpack tuple with structured binding
    Code->>GTest: Return test name string
    GTest->>Test: Register parameterized tests
    
    Note over Test,GTest: Tests run with cleaner,<br/>more readable code

Loading

[greptile-apps]

_{No files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}