diff --git a/CHANGELOG.md b/CHANGELOG.md index 878dad8aa..bc4b8ff54 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,10 @@ This release is WIP - Fix a missing return statement for the `buffer_snapshot` move assignment operator (#335) - Re-enable Windows support by fixing a range of Windows-specific and dependency-related issues (#336) +### Internal + +- Adapt tests for Windows by disabling implementation-dependent graph printing tests and increasing tolerances for executor timing tests (#337) + ## [0.7.0] - 2025-08-18 This release includes changes that may require adjustments when upgrading: diff --git a/test/accessor_tests.cc b/test/accessor_tests.cc index 66d885991..eff797ed9 100644 --- a/test/accessor_tests.cc +++ b/test/accessor_tests.cc @@ -558,6 +558,10 @@ namespace detail { #if CELERITY_ACCESSOR_BOUNDARY_CHECK SKIP("no accessor size guarantees when CELERITY_ACCESSOR_BOUNDARY_CHECK=1."); #endif +#if defined(__clang__) && defined(_MSC_VER) + SKIP("clang-cl does not correctly collapse chained/nested [[msvc::no_unique_address]] members, unlike real MSVC " + "(https://github.com/llvm/llvm-project/issues/143245), so 0-dimensional accessors are not pointer-sized on this compiler."); +#endif // these checks are not static_asserts because they depend on an (optional) compiler layout optimization CHECK(sizeof(accessor) == sizeof(int*)); diff --git a/test/device_selection_tests.cc b/test/device_selection_tests.cc index acf1c766d..f81ac22cb 100644 --- a/test/device_selection_tests.cc +++ b/test/device_selection_tests.cc @@ -1,3 +1,4 @@ +#include #include #include #include diff --git a/test/executor_tests.cc b/test/executor_tests.cc index 8e0f17f0a..a77afef18 100644 --- a/test/executor_tests.cc +++ b/test/executor_tests.cc @@ -970,21 +970,34 @@ TEST_CASE("live_executor emits progress warning when a task appears stuck", "[ex CHECK(test_utils::log_contains_substring(log_level::warn, ", might be stuck. Active instructions: I")); } +namespace { +#if defined(_WIN32) // Windows tends to be a bit flaky with timing, so we give it a bit more leeway +constexpr auto tolerance = std::chrono::milliseconds(20); +#elif defined(__APPLE__) +constexpr auto tolerance = std::chrono::milliseconds(10); +#else // Linux and other Unix-like systems +constexpr auto tolerance = std::chrono::milliseconds(10); +#endif +constexpr auto expected_delay = std::chrono::milliseconds(100); +constexpr auto lower_bound = std::chrono::milliseconds(expected_delay - tolerance); +constexpr auto upper_bound = std::chrono::milliseconds(expected_delay + tolerance); +} // namespace + TEST_CASE("live_executor tracks starvation time", "[executor]") { executor_test_context ectx(executor_type::live); SECTION("starvation time is only recorded while scheduler is busy") { ectx.init(); ectx.notify_scheduler_idle(false); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); // counts since scheduler is busy + std::this_thread::sleep_for(expected_delay); // counts since scheduler is busy ectx.notify_scheduler_idle(true); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); // does not count + std::this_thread::sleep_for(expected_delay); // does not count const auto st = ectx.get_starvation_time(); ectx.finish(); // We have to include some tolerance here b/c we don't know when executor received idle state changes // 20-30us should suffice, but let's err on the side of caution - CHECK(st > std::chrono::milliseconds(90)); - CHECK(st < std::chrono::milliseconds(110)); + CHECK(st > lower_bound); + CHECK(st < upper_bound); } SECTION("scheduler is assumed to be idle initially") { @@ -1000,11 +1013,11 @@ TEST_CASE("live_executor tracks active time", "[executor]") { executor_test_context ectx(executor_type::live); ectx.init(); - ectx.delay(std::chrono::milliseconds(100)); // counts + ectx.delay(expected_delay); // counts ectx.barrier(); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); // doesn't count + std::this_thread::sleep_for(expected_delay); // doesn't count const auto at = ectx.get_active_time(); ectx.finish(); - CHECK(at > std::chrono::milliseconds(90)); - CHECK(at < std::chrono::milliseconds(110)); + CHECK(at > lower_bound); + CHECK(at < upper_bound); } diff --git a/test/grid_test_utils.cc b/test/grid_test_utils.cc index 76f313e99..27195c94b 100644 --- a/test/grid_test_utils.cc +++ b/test/grid_test_utils.cc @@ -1,6 +1,7 @@ #include "grid_test_utils.h" #include "test_utils.h" +#include #include #if CELERITY_DETAIL_HAVE_CAIRO diff --git a/test/print_graph_tests.cc b/test/print_graph_tests.cc index a3603a688..95b176df3 100644 --- a/test/print_graph_tests.cc +++ b/test/print_graph_tests.cc @@ -13,7 +13,19 @@ using namespace celerity::test_utils; namespace acc = celerity::access; +namespace { +#ifdef _WIN32 // Windows' std::unordered_map sorts differently than other standard library implementations, which causes the printed graph to differ. Since the + // tests at the moment rely on exact string matching, we disable them on Windows until we have a better solution. +#define SKIP_WIN32_GRAPH_PRINT_UNSUPPORTED() \ + SKIP("At the moment windows graph printing tests are disabled due to issues with the difference in unordered_map between MSVC and other standard library " \ + "implementations."); +#else +#define SKIP_WIN32_GRAPH_PRINT_UNSUPPORTED() +#endif +} // namespace + TEST_CASE("task-graph printing is unchanged", "[print_graph][task-graph]") { + SKIP_WIN32_GRAPH_PRINT_UNSUPPORTED(); auto tt = test_utils::task_test_context{}; auto range = celerity::range<1>(64); @@ -61,6 +73,7 @@ int count_occurences(const std::string& str, const std::string& substr) { } // namespace TEST_CASE("command-graph printing is unchanged", "[print_graph][command-graph]") { + SKIP_WIN32_GRAPH_PRINT_UNSUPPORTED(); const size_t num_nodes = 4; cdag_test_context cctx(num_nodes); @@ -122,6 +135,7 @@ TEST_CASE("command-graph printing is unchanged", "[print_graph][command-graph]") } TEST_CASE("instruction-graph printing is unchanged", "[print_graph][instruction-graph]") { + SKIP_WIN32_GRAPH_PRINT_UNSUPPORTED(); const size_t num_nodes = 2; const node_id local_nid = 1; const size_t num_local_devices = 2; @@ -290,6 +304,7 @@ TEST_CASE_METHOD(test_utils::runtime_fixture, "buffer debug names show up in the } TEST_CASE_METHOD(test_utils::runtime_fixture, "full graph is printed if CELERITY_PRINT_GRAPHS is set", "[print_graph]") { + SKIP_WIN32_GRAPH_PRINT_UNSUPPORTED(); env::scoped_test_environment tenv(print_graphs_env_setting); queue q;