Skip to content

Comments

Replace GTest with custom framework, add performance test filtering and coverage#744

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

Replace GTest with custom framework, add performance test filtering and coverage#744
Copilot wants to merge 45 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

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>
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@chhwang
Copy link
Contributor

chhwang commented Feb 20, 2026

/azp run mscclpp-ut

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@chhwang chhwang mentioned this pull request Feb 20, 2026
@chhwang
Copy link
Contributor

chhwang commented Feb 23, 2026

/azp run mscclpp-ut

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@chhwang
Copy link
Contributor

chhwang commented Feb 23, 2026

/azp run mscclpp-ut

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@chhwang
Copy link
Contributor

chhwang commented Feb 24, 2026

/azp run mscclpp-ut

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@chhwang chhwang marked this pull request as draft February 24, 2026 00:50
@chhwang chhwang marked this pull request as ready for review February 24, 2026 00:51
@chhwang
Copy link
Contributor

chhwang commented Feb 24, 2026

/azp run mscclpp-ut

@azure-pipelines
Copy link

Azure Pipelines failed to run 1 pipeline(s).

@chhwang
Copy link
Contributor

chhwang commented Feb 24, 2026

/azp run mscclpp-ut

@azure-pipelines
Copy link

Azure Pipelines failed to run 1 pipeline(s).

@chhwang
Copy link
Contributor

chhwang commented Feb 24, 2026

/azp run mscclpp-ut

@azure-pipelines
Copy link

Azure Pipelines failed to run 1 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.

3 participants