Skip to content

Remove GTest dependency, add code coverage, and refactor unit tests and CI pipelines#744

Open
Copilot wants to merge 79 commits intomainfrom
copilot/remove-gtest-use-custom-framework
Open

Remove GTest dependency, add code coverage, and refactor unit tests and CI pipelines#744
Copilot wants to merge 79 commits intomainfrom
copilot/remove-gtest-use-custom-framework

Conversation

Copy link
Contributor

Copilot AI commented Feb 11, 2026

  • Removes the GTest dependency, replacing it with a minimal custom framework (test/framework.*) that covers only what the tests actually use — a unified TEST() macro with SFINAE-based fixture auto-detection, EXPECT_*/ASSERT_* assertions, environments, and setup/teardown.
  • --exclude-perf-tests flag and substring-based negative filtering
  • MSCCLPP_ENABLE_COVERAGE CMake option with gcov/lcov; CI uploads to Codecov
  • Merges standalone test/perf/ into main test targets
  • Refactors Azure pipelines to reduce redundancies & make more readable

Copilot AI and others added 6 commits February 11, 2026 00:17
Co-authored-by: chhwang <8018170+chhwang@users.noreply.github.com>
- Modified test/mp_unit/mp_unit_tests.hpp to use ../framework.hpp instead of gtest/gtest.h
- Enhanced test/framework.hpp with GTest-compatible APIs:
  - Added Environment base class for global test setup/teardown
  - Added TestInfo and UnitTest classes for test metadata access
  - Added GTEST_SKIP macro support via SkipHelper class
  - Added namespace alias 'testing' for compatibility
  - Added InitGoogleTest and AddGlobalTestEnvironment helper functions
- Updated test/framework.cc with implementations for new classes
- All mp_unit test files now use framework.hpp through mp_unit_tests.hpp
- Formatting applied via lint.sh

Co-authored-by: chhwang <8018170+chhwang@users.noreply.github.com>
- Remove duplicate static getMPIRank() and getMPISize() functions
- Add full namespace qualification to GTEST_SKIP macro

Co-authored-by: chhwang <8018170+chhwang@users.noreply.github.com>
- Move test framework from test/perf/ to test/ for shared use
- Add GTest-compatible macros (TEST, TEST_F, EXPECT_*, ASSERT_*, etc.)
- Remove GTest dependency from CMakeLists.txt
- Add test_framework library for unit and mp_unit tests
- Add code coverage support with lcov (MSCCLPP_ENABLE_COVERAGE option)
- Update perf tests to use shared framework

Co-authored-by: chhwang <8018170+chhwang@users.noreply.github.com>
Support --gtest_filter command line argument for test filtering,
compatible with Azure pipeline configurations.

Co-authored-by: chhwang <8018170+chhwang@users.noreply.github.com>
- Move PerfTestResult struct definition outside vector declaration
- Move getCurrentTimestamp to anonymous namespace
- Add documentation for GTEST_SKIP macro explaining RAII pattern

Co-authored-by: chhwang <8018170+chhwang@users.noreply.github.com>
Copilot AI changed the title [WIP] Remove GTest and implement custom testing framework Replace GTest with custom framework and add code coverage Feb 11, 2026
Copilot AI requested a review from chhwang February 11, 2026 00:30
The recent removal of GTest and introduction of custom test framework
requires MPI dependency which is not needed for CodeQL analysis.
Disable test building in CodeQL workflows to fix the build failures.

CodeQL only needs to analyze the core library code, not the tests.

Co-authored-by: chhwang <8018170+chhwang@users.noreply.github.com>
Copilot AI changed the title Replace GTest with custom framework and add code coverage Fix CodeQL workflows by disabling test builds Feb 11, 2026
Copilot AI and others added 3 commits February 11, 2026 01:02
Co-authored-by: chhwang <8018170+chhwang@users.noreply.github.com>
- Make MPI REQUIRED for test builds (clearer error messages)
- Add project include directories to test_framework library
- Fix core_tests.cc to use custom framework correctly
- Fix mp_unit_tests.hpp to use mscclpp::test namespace
- Add FAIL() macro with streaming support for test messages
- Building tests now works in Docker environment with GPU bypass

Tests can now be built using:
docker run --rm -v $(pwd):/workspace -w /workspace \
  ghcr.io/microsoft/mscclpp/mscclpp:base-dev-cuda12.4 bash -c \
  "mkdir build && cd build && cmake -DMSCCLPP_BYPASS_GPU_CHECK=ON \
   -DMSCCLPP_USE_CUDA=ON .. && make -j"

Co-authored-by: chhwang <8018170+chhwang@users.noreply.github.com>
- Move helper classes inside namespace for proper access
- Remove duplicate class definitions outside namespace
- Tests can now build in Docker with CUDA toolkit installed
- Remaining issues: ErrorCode and TransportFlags need operator<< for EXPECT_EQ

Successfully building with Docker:
docker run --rm -v $(pwd):/workspace -w /workspace \
  ghcr.io/microsoft/mscclpp/mscclpp:base-dev-cuda12.4 bash -c \
  "mkdir build && cd build && cmake -DMSCCLPP_BYPASS_GPU_CHECK=ON \
   -DMSCCLPP_USE_CUDA=ON .. && make -j4"

Note: Some unit tests (errors_tests.cc, core_tests.cc) need operator<<
defined for ErrorCode and TransportFlags to compile with custom framework.

Co-authored-by: chhwang <8018170+chhwang@users.noreply.github.com>
Copilot AI changed the title Fix CodeQL workflows by disabling test builds Fix test builds with GPU bypass in Docker environment Feb 11, 2026
- Remove build_test/ directory containing CMake cache and build files
- Update .gitignore to exclude build_*/ pattern to prevent future accidents

These CMake artifacts (CMakeCache.txt, CMakeFiles/, generated headers)
were accidentally committed and should never be in version control.

Co-authored-by: chhwang <8018170+chhwang@users.noreply.github.com>
Copilot AI changed the title Fix test builds with GPU bypass in Docker environment Remove CMake build artifacts from repository Feb 11, 2026
Copilot AI and others added 2 commits February 11, 2026 02:21
- Add nlohmann::ordered_json metrics field to TestResult struct
- Add nlohmann/json.hpp include to test/framework.hpp
- Link test_framework with nlohmann_json::nlohmann_json
- Replace PerfTestResult with TestResult in test/perf/framework.cc
- Move perf utility functions to utils namespace for consistency
- Remove duplicate PerfTestResult struct definition

This consolidates the two similar structs into one, reducing code
duplication while maintaining all necessary fields for both unit
tests (passed/failure_message) and performance tests (metrics).

Verified build succeeds with Docker:
docker run --rm -v $(pwd):/workspace -w /workspace \
  ghcr.io/microsoft/mscclpp/mscclpp:base-dev-cuda12.4 bash -c \
  "cd /workspace/build && make -j4 fifo_test"

Co-authored-by: chhwang <8018170+chhwang@users.noreply.github.com>
Co-authored-by: chhwang <8018170+chhwang@users.noreply.github.com>
@chhwang
Copy link
Contributor

chhwang commented Mar 17, 2026

/azp run mscclpp-codecov

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@chhwang
Copy link
Contributor

chhwang commented Mar 17, 2026

/azp run mscclpp-codecov

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@chhwang
Copy link
Contributor

chhwang commented Mar 18, 2026

/azp run mscclpp-codecov

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@chhwang
Copy link
Contributor

chhwang commented Mar 18, 2026

/azp run

@chhwang chhwang requested a review from Copilot March 18, 2026 02:33
@azure-pipelines
Copy link

Azure Pipelines successfully started running 5 pipeline(s).

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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

Address review comments:
1. Ensure TearDown() is always called if SetUp() succeeds, even when
   TestBody() throws. This prevents resource leaks and maintains MPI
   synchronization between tests.
2. Replace assert() in fifo_perf_tests.cu with proper return false
   on validation failure, ensuring consistent test failure reporting.

Fixes:
- test/framework.cc: Track SetUp success and call TearDown in finally-style
- test/unit/fifo_perf_tests.cu: Replace assert with explicit check

Co-authored-by: chhwang <8018170+chhwang@users.noreply.github.com>
@chhwang
Copy link
Contributor

chhwang commented Mar 18, 2026

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 5 pipeline(s).

@chhwang chhwang requested a review from a team March 18, 2026 20:32
@chhwang
Copy link
Contributor

chhwang commented Mar 18, 2026

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 4 pipeline(s).

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.

5 participants