Make simple kernel timer stdout in ascii text with unit-tests by Copilot · Pull Request #328 · kokkos/kokkos-tools

Copilot · 2026-03-15T21:26:57Z

Ports print_ascii from PR #323 and adds death tests validating kokkosp_init_library and kokkosp_finalize_library for the simple-kernel-timer tool.

`kp_kernel_timer.cpp` changes (PR #323 port)

Added print_ascii() — formats kernel/region timing to stdout as default output
kokkosp_finalize_library() now calls print_ascii() unless KOKKOS_TOOLS_TIMER_BINARY or KOKKOS_TOOLS_TIMER_JSON is set

7 death tests in `tests/sampler/test_simple_kernel_timer.cpp`

All tests are consolidated into a single file using a SimpleKernelTimer_DeathTest test fixture with TEST_F. Each test runs Kokkos::initialize() → kernel work → Kokkos::finalize() in a child process via EXPECT_EXIT, redirecting stdout→stderr with dup2 for capture. Uses ::testing::AllOf to validate multiple patterns per subprocess and ::testing::FLAGS_gtest_death_test_style = "threadsafe" at the start of each test.

Note: dup2(STDERR_FILENO, STDOUT_FILENO) remains inside each EXPECT_EXIT block because SetUp() runs in the parent process while EXPECT_EXIT forks a child — moving it to the fixture would not affect the child's file descriptors.

Test case	Coverage
`parfor`	`parallel_for`
`parreduce`	`parallel_reduce`
`parscan`	`parallel_scan`
`region`	Region
`parfor_region`	`parallel_for` + Region
`parreduce_region`	`parallel_reduce` + Region
`parscan_region`	`parallel_scan` + Region

Each test validates:

Init message present
Correct kernel type labels (ParFor, ParRed, ParScan, Region)
Non-negative timing values via regex ([0-9]+\.[0-9]+)
Expected call counts in summary

TEST_F(SimpleKernelTimer_DeathTest, parfor) {
  ::testing::FLAGS_gtest_death_test_style = "threadsafe";
  EXPECT_EXIT(
      {
        dup2(STDERR_FILENO, STDOUT_FILENO);
        Kokkos::initialize();
        for (int i = 0; i < 5; i++)
          Kokkos::parallel_for("named kernel", Kokkos::RangePolicy<>(0, 1), ParForFunctor{});
        Kokkos::finalize();
        exit(0);
      },
      ::testing::ExitedWithCode(0),
      ::testing::AllOf(
          HasSubstr("KokkosP: Simple Kernel Timer Library Initialized"),
          ContainsRegex("named kernel.*\\(ParFor\\)"),
          ContainsRegex("Total Calls to Kokkos Kernels:[ ]+5"),
          ContainsRegex("\\(ParFor\\)[ ]+[0-9]+\\.[0-9]+[ ]+5[ ]+[0-9]+\\.[0-9]+")));
}

CMake

Single test registered via kp_add_executable_and_test with kp_kernel_timer as the tool library:

kp_add_executable_and_test(
    TARGET_NAME       test_simple_kernel_timer
    SOURCE_FILE       test_simple_kernel_timer.cpp
    KOKKOS_TOOLS_LIBS kp_kernel_timer
)

💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.

…alize_library - Port print_ascii function from PR #323 to simple-kernel-timer - Modify kokkosp_finalize_library to use print_ascii by default - Add 7 death test files covering parallel_for, parallel_reduce, parallel_scan, region, and combinations with regions - All 17 tests pass (10 existing + 7 new) Co-authored-by: yasahi-hpc <57478230+yasahi-hpc@users.noreply.github.com>

Each test now uses a single EXPECT_EXIT with ::testing::AllOf to validate all assertions in one child process, instead of repeating the setup for each individual assertion. Co-authored-by: yasahi-hpc <57478230+yasahi-hpc@users.noreply.github.com>

yasahi-hpc

Merge all the proposed tests (test_parfor_init_finalize.cpp‎, test_parreduce_init_finalize.cpp‎, test_parscan_init_finalize.cpp‎, test_region_init_finalize.cpp, test_parfor_region_init_finalize.cpp, , test_parreduce_region_init_finalize.cpp, test_parscan_region_init_finalize.cpp) into a single test file named "test_simple_kernel_timer.cpp". CMake should be updated accordingly.
Investigate the possible to introduce a single test fixture that performs dup2(STDERR_FILENO, STDOUT_FILENO); in SetUp() and remove dup2(STDERR_FILENO, STDOUT_FILENO); inside each tests. If this does not change the behavior, apply this change.
Introduce ::testing::FLAGS_gtest_death_test_style = "threadsafe"; at the first line of each test.
Confirm that tests pass after changes

yasahi-hpc · 2026-03-15T21:55:38Z

 )
+
+## tests for simple kernel timer init/finalize with print_ascii output
+


Please merge all the proposed tests into a single test file named "test_simple_kernel_timer"

Done in db16c5f — all tests merged into test_simple_kernel_timer.cpp with a single test_simple_kernel_timer CMake target.

yasahi-hpc · 2026-03-15T21:57:12Z

+ *       The print_ascii output is captured and validated.
+ */
+
+TEST(SimpleKernelTimerParFor_DeathTest, parfor) {


Add ::testing::FLAGS_gtest_death_test_style = "threadsafe";. This should be added for all the proposed test functions.

Added to all 7 test functions in db16c5f.

yasahi-hpc · 2026-03-15T21:58:07Z

 *~
 *.so
 /src/tools/simple-kernel-timer/kp_reader
+build/


These changes can be made in another PR

Reverted in db16c5f.

yasahi-hpc · 2026-03-15T22:01:07Z

+  EXPECT_EXIT(
+      {
+        // Redirect stdout to stderr so the death test captures the output.
+        dup2(STDERR_FILENO, STDOUT_FILENO);


If it does not change the behavior, I would like a single test fixture that performs dup2(STDERR_FILENO, STDOUT_FILENO); in SetUp() and then reuse that for all the proposed tests. Apply this change only if the behavior does not change

Investigated — moving dup2 to SetUp() does change the behavior. SetUp() runs in the parent process, while EXPECT_EXIT forks a child process (especially with "threadsafe" style which re-execs). The dup2 redirect must happen inside the child's EXPECT_EXIT block. Added a SimpleKernelTimer_DeathTest fixture with SetUp() documenting this constraint. See db16c5f.

- Merge all 7 test files into test_simple_kernel_timer.cpp - Use SimpleKernelTimer_DeathTest test fixture with TEST_F - Add ::testing::FLAGS_gtest_death_test_style = "threadsafe" to each test - Document in fixture why dup2 must stay inside EXPECT_EXIT block - Update CMakeLists.txt to use single test target - Revert .gitignore changes (to be done in another PR) Co-authored-by: yasahi-hpc <57478230+yasahi-hpc@users.noreply.github.com>

Co-authored-by: yasahi-hpc <57478230+yasahi-hpc@users.noreply.github.com>

Copilot · 2026-03-15T22:14:55Z

Merge all the proposed tests (test_parfor_init_finalize.cpp‎, test_parreduce_init_finalize.cpp‎, test_parscan_init_finalize.cpp‎, test_region_init_finalize.cpp, `test_parfor_region_in...

All 4 items addressed in db16c5f:

Merged all 7 test files into test_simple_kernel_timer.cpp, CMake updated to single target.
Investigated the fixture approach — dup2 cannot be moved to SetUp() because SetUp() runs in the parent process while EXPECT_EXIT forks a child process. The redirect must happen in the child. Added SimpleKernelTimer_DeathTest fixture with documentation explaining this.
Added ::testing::FLAGS_gtest_death_test_style = "threadsafe"; as the first line of each test.
All 11 tests pass (10 existing + 1 new with 7 test cases).

JBludau · 2026-03-17T15:54:39Z

I have trouble understanding the title. Are these not two topics mixed into one pr?

yasahi-hpc · 2026-03-17T16:31:30Z

I have trouble understanding the title. Are these not two topics mixed into one pr?

Sorry for confusion, I have rephrased.
A bit of context, I initially made #323, but it is claimed that tests are needed

Adrien-Tab · 2026-03-26T17:14:43Z


  if(NULL != outputDelimiter) {
          free(outputDelimiter);
  }*/


This comment can be deleted

It makes sense, but I would like to keep the changes minimal.
We can make refactor/cleanup PR after this one

Adrien-Tab · 2026-03-30T09:28:01Z

This supersedes #323

Adrien-Tab

Maybe an option to print in ascii to a file, like google Benchmark do ?

masterleinad

Fine with me.

Initial plan

3c5bf18

Copilot AI assigned Copilot and yasahi-hpc Mar 15, 2026

Copilot started work on behalf of yasahi-hpc March 15, 2026 21:27 View session

Copilot AI and others added 2 commits March 15, 2026 21:39

Copilot AI changed the title ~~[WIP] Add unit tests for kokkosp_init_library and kokkosp_finalize_library~~ Add unit tests for kokkosp_init_library/kokkosp_finalize_library with print_ascii output Mar 15, 2026

Copilot AI requested a review from yasahi-hpc March 15, 2026 21:44

Copilot finished work on behalf of yasahi-hpc March 15, 2026 21:44

yasahi-hpc requested changes Mar 15, 2026

View reviewed changes

Copilot started work on behalf of yasahi-hpc March 15, 2026 22:08 View session

Copilot AI and others added 2 commits March 15, 2026 22:13

Remove accidentally committed build directory from tracking

db16c5f

Co-authored-by: yasahi-hpc <57478230+yasahi-hpc@users.noreply.github.com>

Copilot AI requested a review from yasahi-hpc March 15, 2026 22:15

Copilot finished work on behalf of yasahi-hpc March 15, 2026 22:15

yasahi-hpc approved these changes Mar 15, 2026

View reviewed changes

yasahi-hpc marked this pull request as ready for review March 15, 2026 22:17

apply clangformat

7588c7a

yasahi-hpc changed the title ~~Add unit tests for kokkosp_init_library/kokkosp_finalize_library with print_ascii output~~ Make simple kernel timer stdout in ascii text with unit-tests Mar 17, 2026

Adrien-Tab reviewed Mar 26, 2026

View reviewed changes

Adrien-Tab approved these changes Mar 30, 2026

View reviewed changes

masterleinad approved these changes Mar 30, 2026

View reviewed changes

		)

		## tests for simple kernel timer init/finalize with print_ascii output

Conversation

Copilot AI commented Mar 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

kp_kernel_timer.cpp changes (PR #323 port)

7 death tests in tests/sampler/test_simple_kernel_timer.cpp

CMake

Uh oh!

yasahi-hpc left a comment

Choose a reason for hiding this comment

Uh oh!

yasahi-hpc Mar 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 15, 2026

Choose a reason for hiding this comment

Uh oh!

yasahi-hpc Mar 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 15, 2026

Choose a reason for hiding this comment

Uh oh!

yasahi-hpc Mar 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 15, 2026

Choose a reason for hiding this comment

Uh oh!

yasahi-hpc Mar 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Mar 15, 2026

Uh oh!

JBludau commented Mar 17, 2026

Uh oh!

yasahi-hpc commented Mar 17, 2026

Uh oh!

Adrien-Tab Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

yasahi-hpc Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Adrien-Tab commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Adrien-Tab left a comment

Choose a reason for hiding this comment

Uh oh!

masterleinad left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Copilot AI commented Mar 15, 2026 •

edited

Loading

`kp_kernel_timer.cpp` changes (PR #323 port)

7 death tests in `tests/sampler/test_simple_kernel_timer.cpp`

Adrien-Tab commented Mar 30, 2026 •

edited

Loading