fix(tests): make two Windows CI failures deterministic (Nightly CI #8837) (#8840)

* fix(tests): reject overlay directories before opening on all platforms

DatanodeOverlay::load() opened the target before checking is_file(). On
Unix, File::open on a directory succeeds and the loader rejects it with
"must be a regular file". On Windows, File::open on a directory fails up
front with "Access is denied", so the type check was never reached and
the rejects_directories_and_parse_errors test failed 4/4 in Nightly CI
(issue #8837).

Check std::fs::metadata before File::open: metadata succeeds on
directories on both platforms, so the error message is now identical
everywhere and the test assertion holds on Windows too.

Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>

* test(datanode): make test_region_error deterministic across platforms

The second phase raced a 100ms mock handle delay against a 200ms
replay_timeout; on busy Windows CI runners the error could land after the
timeout fired, flaking reply.error.is_some() (Nightly CI, issue #8837).
Use a mock handle that returns the error on its first poll with no delay:
the catchup future completes before replay_timeout can ever fire, so the
test no longer depends on wall-clock scheduling.

Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>

---------

Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>

src/datanode/src/heartbeat/handler/upgrade_region.rs

@@ -408,9 +408,12 @@ mod tests {
     #[tokio::test]
     async fn test_region_error() {
         common_telemetry::init_default_ut_logging();
-        let mock_region_server = mock_region_server();
         let region_id = RegionId::new(1024, 1);
 
+        // With the default (zero) `replay_timeout`, the handler times out before the
+        // mock handle's 100ms delay elapses: it didn't wait for handle returns; it
+        // had no idea about the error.
+        let region_server = mock_region_server();
         let (mock_engine, _) =
             MockRegionEngine::with_custom_apply_fn(MITO_ENGINE_NAME, |region_engine| {
                 // Region is not ready.
@@ -424,9 +427,9 @@ mod tests {
                 // Note: Don't change.
                 region_engine.handle_request_delay = Some(Duration::from_millis(100));
             });
-        mock_region_server.register_test_region(region_id, mock_engine);
+        region_server.register_test_region(region_id, mock_engine);
         let kv_backend = Arc::new(MemoryKvBackend::new());
-        let handler_context = HandlerContext::new_for_test(mock_region_server, kv_backend);
+        let handler_context = HandlerContext::new_for_test(region_server, kv_backend);
         let reply = UpgradeRegionsHandler::new_test()
             .handle(
                 &handler_context,
@@ -437,12 +440,32 @@ mod tests {
             )
             .await;
 
-        // It didn't wait for handle returns; it had no idea about the error.
         let reply = &reply.unwrap().expect_upgrade_regions_reply()[0];
         assert!(!reply.ready);
         assert!(reply.exists);
         assert!(reply.error.is_none());
 
+        // With a 200ms `replay_timeout` and a mock handle that returns the error
+        // immediately (no delay), the handler waits for the handle and propagates
+        // its error deterministically: the catchup future completes on its first
+        // poll, so the `replay_timeout` can never fire first. This avoids racing
+        // real wall-clock time (a 100ms delay vs 200ms timeout is flaky on
+        // Windows CI).
+        let mock_region_server = mock_region_server();
+        let (mock_engine, _) =
+            MockRegionEngine::with_custom_apply_fn(MITO_ENGINE_NAME, |region_engine| {
+                // Region is not ready.
+                region_engine.mock_role = Some(Some(RegionRole::Follower));
+                region_engine.handle_request_mock_fn = Some(Box::new(|_, _| {
+                    error::UnexpectedSnafu {
+                        violated: "mock_error".to_string(),
+                    }
+                    .fail()
+                }));
+            });
+        mock_region_server.register_test_region(region_id, mock_engine);
+        let kv_backend = Arc::new(MemoryKvBackend::new());
+        let handler_context = HandlerContext::new_for_test(mock_region_server, kv_backend);
         let reply = UpgradeRegionsHandler::new_test()
             .handle(
                 &handler_context,

tests/runner/src/cmd/datanode_overlay.rs

@@ -106,13 +106,11 @@ impl DatanodeOverlay {
             ));
         }
 
-        let mut file = File::open(&canonical_target).map_err(|error| {
-            format!(
-                "Failed to open datanode overlay {}: {error}",
-                canonical_target.display()
-            )
-        })?;
-        let metadata = file.metadata().map_err(|error| {
+        // Check the type before opening so the error is identical on every
+        // platform: `std::fs::metadata` succeeds on directories on both Unix
+        // and Windows, whereas `File::open` on a directory fails up front on
+        // Windows (with a platform-specific message).
+        let metadata = std::fs::metadata(&canonical_target).map_err(|error| {
             format!(
                 "Failed to inspect datanode overlay {}: {error}",
                 canonical_target.display()
@@ -125,6 +123,13 @@ impl DatanodeOverlay {
             ));
         }
 
+        let mut file = File::open(&canonical_target).map_err(|error| {
+            format!(
+                "Failed to open datanode overlay {}: {error}",
+                canonical_target.display()
+            )
+        })?;
+
         let mut content = String::new();
         file.read_to_string(&mut content).map_err(|error| {
             format!(
@@ -613,7 +618,10 @@ mod tests {
 
         let directory_error =
             DatanodeOverlay::load(temp_dir.path(), Path::new("directory.toml")).unwrap_err();
-        assert!(directory_error.contains("regular file"));
+        assert!(
+            directory_error.contains("regular file"),
+            "unexpected error for directory.toml: {directory_error}"
+        );
         let parse_error =
             DatanodeOverlay::load(temp_dir.path(), Path::new("broken.toml")).unwrap_err();
         assert!(parse_error.contains("Failed to parse datanode overlay"));