Running "dart test" inside "core/pkgs/path" has race condition and sporadic failures

[jensjoha]

Running dart test inside "core/pkgs/path" on my machine fails several time per 10 runs.

Obviously the concurrency matters - for me -j defaults to 6.

Applying this change makes the thing happen more often (for me, like this it happens every time):

diff --git a/pkgs/path/test/io_test.dart b/pkgs/path/test/io_test.dart
index e428b89a..c51ca7ab 100644
--- a/pkgs/path/test/io_test.dart
+++ b/pkgs/path/test/io_test.dart
@@ -97,8 +97,12 @@ void main() {
     () {
       final dir = path.current;
       try {
+        io.sleep(const Duration(seconds: 2));
+
         io.Directory.current = path.rootPrefix(path.current);
 
+        io.sleep(const Duration(seconds: 2));
+
         expect(
           path.relative(path.absolute('foo/bar'), from: path.current),
           path.relative(path.absolute('foo/bar')),
diff --git a/pkgs/path/test/path_map_test.dart b/pkgs/path/test/path_map_test.dart
index 11c4a3e0..8b9f84b7 100644
--- a/pkgs/path/test/path_map_test.dart
+++ b/pkgs/path/test/path_map_test.dart
@@ -2,6 +2,8 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
+import 'dart:io';
+
 import 'package:path/path.dart';
 import 'package:test/test.dart';
 
@@ -70,10 +72,19 @@ void main() {
     });
 
     test('uses the second value in the case of duplicates', () {
-      final map = PathMap.of({'foo': 1, absolute('foo'): 2});
+      final orgPath = Directory.current.path;
+      final absolute2 = absolute('fooSpecial');
+      if (Directory.current.path != orgPath) {
+        // ignore: only_throw_errors
+        throw 'Who knows which it used.';
+      }
+      while(Directory.current.path == orgPath) {
+        sleep(const Duration(milliseconds: 1));
+      }
+      final map = PathMap.of({'fooSpecial': 1, absolute2: 2});
       expect(map, hasLength(1));
-      expect(map, containsPair('foo', 2));
-      expect(map, containsPair(absolute('foo'), 2));
+      expect(map, containsPair('fooSpecial', 2));
+      expect(map, containsPair(absolute('fooSpecial'), 2));
     });
   });
 }

(I run dart test -j2 - but it probably just has to be j>1).

[jakemac53]

This is almost certainly because of the test/io_test.dart file changing the current working directory.

I don't really see any way to resolve that other than using -j 1 for this package.

[jensjoha]

Doing something like

      var savedCurrentDirectory = io.Directory.current;
      io.IOOverrides.runZoned(
          () {
[...]
          },
          getCurrentDirectory: () => savedCurrentDirectory,
          setCurrentDirectory: (dir) {
            savedCurrentDirectory = io.Directory(dir);
          });

should (and seems to) fix it.

This would also allow the test to get rid of the try/finally.

One could even make a helper method and fix it like this:

diff --git a/pkgs/path/test/io_test.dart b/pkgs/path/test/io_test.dart
index e428b89a..a26c4873 100644
--- a/pkgs/path/test/io_test.dart
+++ b/pkgs/path/test/io_test.dart
@@ -43,9 +43,7 @@ void main() {
 
     test(
       'uses the previous working directory if deleted',
-      () {
-        final dir = io.Directory.current.path;
-        try {
+      currentDirHelper(() {
         final temp = io.Directory.systemTemp.createTempSync('path_test');
         final tempPath = temp.resolveSymbolicLinksSync();
         io.Directory.current = temp;
@@ -57,18 +55,14 @@ void main() {
 
         // Even though the directory no longer exists, no exception is thrown.
         expect(path.normalize(path.absolute(path.current)), equals(tempPath));
-        } finally {
-          io.Directory.current = dir;
-        }
-      },
+      }),
       //TODO: Figure out why this is failing on windows and fix!
       skip: io.Platform.isWindows ? 'Untriaged failure on Windows' : false,
     );
   });
 
-  test('registers changes to the working directory', () {
+  test('registers changes to the working directory', currentDirHelper(() {
     final dir = io.Directory.current.path;
-    try {
     expect(path.absolute('foo/bar'), equals(path.join(dir, 'foo/bar')));
     expect(
       path.absolute('foo/bar'),
@@ -84,20 +78,17 @@ void main() {
       path.normalize(path.absolute('foo/bar')),
       equals(path.normalize(path.context.join(dir, '../foo/bar'))),
     );
-    } finally {
-      io.Directory.current = dir;
-    }
-  });
+  }));
 
   // Regression test for #35. This tests against the *actual* working directory
   // rather than just a custom context because we do some processing in
   // [path.current] that has clobbered the root in the past.
   test(
     'absolute works on root working directory',
-    () {
-      final dir = path.current;
-      try {
+    currentDirHelper(() {
+      io.sleep(const Duration(seconds: 2));
       io.Directory.current = path.rootPrefix(path.current);
+      io.sleep(const Duration(seconds: 2));
 
       expect(
         path.relative(path.absolute('foo/bar'), from: path.current),
@@ -113,11 +104,17 @@ void main() {
         path.normalize(path.absolute('foo/bar')),
         equals(path.normalize(path.join(path.current, '../foo/bar'))),
       );
-      } finally {
-        io.Directory.current = dir;
-      }
-    },
+    }),
     //TODO(kevmoo): figure out why this is failing on windows and fix!
     skip: io.Platform.isWindows ? 'Untriaged failure on Windows' : null,
   );
 }
+
+dynamic Function() currentDirHelper(dynamic Function() body) {
+  var savedCurrentDirectory = io.Directory.current;
+  return () => io.IOOverrides.runZoned(body,
+      getCurrentDirectory: () => savedCurrentDirectory,
+      setCurrentDirectory: (dir) {
+        savedCurrentDirectory = io.Directory(dir);
+      });
+}

In fact I'll see if I can figure out this github PR thing...