chore: fix test_ci_asserts on windows nightlyRr/core 416 test assert nonzero exit code windows (#5696)

Resolves CORE-416.

The `test_ci_asserts` unit test has been causing windows nightly runs to
fail quite mysteriously as `ctest` would report all tests passing but
still return an exit code of `-1` to the runner, failing the job.

It turns out that when the `-V` (verbose) flag is used, `ctest` does
some processing of the stdout/stderr of each unit test. Even if the
tests succeed with an exit status of zero, certain patterns in the
stdout or stderr can cause `ctest` to report failure anyway.

One of those patterns seemingly is `internal error`, which is output by
our `passert` failure. So, when `test_ci_asserts` would spawn a
subprocess which intentionally fails asserts, `internal error` was
printed to stderr and flagged by `ctest`, causing it to report failure.

This pull request fixes that by capturing the output of the
assertion-failing subprocess instead so it doesn't get reported to
stdout.

We also fix the new `passert` callback registration tests, whose
failures we did not notice because they only failed in this nightly job.

---
TYPE: NO_HISTORY
DESC: Fix test_ci_asserts

Author: rroelke

.github/workflows/nightly-test.yml

@@ -57,7 +57,7 @@ jobs:
         if: contains(matrix.os, 'windows')
         working-directory: ${{ matrix.working_directory || github.workspace }}
         run: |
-          cmake -B build -S $env:GITHUB_WORKSPACE -DTILEDB_WERROR=ON -DTILEDB_SERIALIZATION=ON -DCMAKE_BUILD_TYPE=${{ matrix.build_type }}
+          cmake -B build -S $env:GITHUB_WORKSPACE -DTILEDB_WERROR=ON -DTILEDB_SERIALIZATION=ON -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} -DTILEDB_ASSERTIONS=${{ matrix.assertions }}
 
       - name: Build TileDB
         working-directory: ${{ matrix.working_directory || github.workspace }}

test/ci/test_assert.cc

@@ -67,14 +67,52 @@ TEST_CASE("CI: Test assertions configuration", "[ci][assertions]") {
         std::filesystem::temp_directory_path() / "try_assert.log";
     sprintf(
         command,
-        "%s %s",
+        "%s %s 2>&1",
         TILEDB_PATH_TO_TRY_ASSERT,
         try_assert_logfile.value().string().c_str());
   } else {
-    sprintf(command, "%s", TILEDB_PATH_TO_TRY_ASSERT);
+    sprintf(command, "%s 2>&1", TILEDB_PATH_TO_TRY_ASSERT);
   }
 
-  const int retval = system(command);
+#ifdef _WIN32
+  FILE* sub = _popen(command, "r");
+#else
+  FILE* sub = popen(command, "r");
+#endif
+  REQUIRE(sub);
+
+  std::vector<std::string> sub_output;
+  {
+    char linebuf[1024];
+    while (fgets(linebuf, sizeof(linebuf), sub) != nullptr) {
+      sub_output.push_back(std::string(linebuf));
+    }
+  }
+#ifdef _WIN32
+  const int retval = _pclose(sub);
+#else
+  const int retval = pclose(sub);
+#endif
+
+  if (builtWithAssertions) {
+    CHECK(sub_output.size() >= 2);
+    if (sub_output.size() >= 1) {
+      CAPTURE(sub_output[0]);
+      CHECK(
+          sub_output[0].find(
+              "FATAL TileDB core library internal error: false") == 0);
+    }
+    if (sub_output.size() >= 2) {
+      CAPTURE(sub_output[1]);
+      CHECK(sub_output[1].find("try_assert.cc") != std::string::npos);
+    }
+  } else {
+    CHECK(sub_output.size() == 1);
+    if (sub_output.size() >= 1) {
+      CAPTURE(sub_output[0]);
+      CHECK(sub_output[0].find("Assert did not exit!") == 0);
+    }
+  }
 
   // in case value is one not currently accepted, report what was returned.
   std::cout << "retval is " << retval << " (0x" << std::hex << retval
@@ -160,34 +198,54 @@ TEST_CASE("CI: passert callback registration non-FIFO", "[assertions]") {
   REQUIRE(order_.empty());
 
   tiledb::common::passert_failure_run_callbacks();
-  CHECK(order_ == std::vector<int>{10000, 100, 0});
+  if (builtWithAssertions) {
+    CHECK(order_ == std::vector<int>{10000, 100, 0});
+  } else {
+    CHECK(order_ == std::vector<int>{});
+  }
   order_.clear();
 
   tiledb::common::passert_failure_run_callbacks();
-  CHECK(order_ == std::vector<int>{10001, 101, 1});
+  if (builtWithAssertions) {
+    CHECK(order_ == std::vector<int>{10001, 101, 1});
+  } else {
+    CHECK(order_ == std::vector<int>{});
+  }
   order_.clear();
 
   SECTION("Remove x") {
     inc_x.reset();
 
     tiledb::common::passert_failure_run_callbacks();
-    CHECK(order_ == std::vector<int>{10002, 102});
+    if (builtWithAssertions) {
+      CHECK(order_ == std::vector<int>{10002, 102});
+    } else {
+      CHECK(order_ == std::vector<int>{});
+    }
     order_.clear();
   }
 
   SECTION("Remove y") {
     inc_y.reset();
 
     tiledb::common::passert_failure_run_callbacks();
-    CHECK(order_ == std::vector<int>{10002, 2});
+    if (builtWithAssertions) {
+      CHECK(order_ == std::vector<int>{10002, 2});
+    } else {
+      CHECK(order_ == std::vector<int>{});
+    }
     order_.clear();
   }
 
   SECTION("Remove z") {
     inc_z.reset();
 
     tiledb::common::passert_failure_run_callbacks();
-    CHECK(order_ == std::vector<int>{102, 2});
+    if (builtWithAssertions) {
+      CHECK(order_ == std::vector<int>{102, 2});
+    } else {
+      CHECK(order_ == std::vector<int>{});
+    }
     order_.clear();
   }
 }