From cfeaef4c1f7086b1303560cb4eb13607bf08cb6c Mon Sep 17 00:00:00 2001 From: David Morgan Date: Mon, 29 Sep 2025 13:35:43 +0200 Subject: [PATCH] Add test coverage around startup race and missing file on startup. --- .../{shared.dart => file_tests.dart} | 34 ++++++++++++++-- .../test/file_watcher/native_test.dart | 10 ++--- .../test/file_watcher/polling_test.dart | 11 ++---- .../test/file_watcher/startup_race_tests.dart | 39 +++++++++++++++++++ pkgs/watcher/test/utils.dart | 27 +++++++++++-- 5 files changed, 101 insertions(+), 20 deletions(-) rename pkgs/watcher/test/file_watcher/{shared.dart => file_tests.dart} (69%) create mode 100644 pkgs/watcher/test/file_watcher/startup_race_tests.dart diff --git a/pkgs/watcher/test/file_watcher/shared.dart b/pkgs/watcher/test/file_watcher/file_tests.dart similarity index 69% rename from pkgs/watcher/test/file_watcher/shared.dart rename to pkgs/watcher/test/file_watcher/file_tests.dart index 081b92e11f..74980f5f63 100644 --- a/pkgs/watcher/test/file_watcher/shared.dart +++ b/pkgs/watcher/test/file_watcher/file_tests.dart @@ -2,16 +2,24 @@ // 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:test/test.dart'; import '../utils.dart'; -void sharedTests() { +void fileTests({required bool isNative}) { + setUp(() { + writeFile('file.txt'); + }); + test("doesn't notify if the file isn't modified", () async { + // TODO(davidmorgan): fix startup race on MacOS. + if (isNative && Platform.isMacOS) { + await Future.delayed(const Duration(milliseconds: 100)); + } await startWatcher(path: 'file.txt'); - await pumpEventQueue(); - deleteFile('file.txt'); - await expectRemoveEvent('file.txt'); + await expectNoEvents(); }); test('notifies when a file is modified', () async { @@ -70,4 +78,22 @@ void sharedTests() { // startWatcher awaits 'ready' await startWatcher(path: 'foo/bar/baz'); }); + + test('throws if file does not exist', () async { + await startWatcher(path: 'other_file.txt'); + + // TODO(davidmorgan): reconcile differences. + if (isNative && Platform.isLinux) { + expect(expectNoEvents, throwsA(isA())); + } else { + // The polling watcher and the MacOS watcher do not throw on missing file + // on watch. Instead, they report both creating and modification as + // modifications. + await expectNoEvents(); + writeFile('other_file.txt'); + await expectModifyEvent('other_file.txt'); + writeFile('other_file.txt'); + await expectModifyEvent('other_file.txt'); + } + }); } diff --git a/pkgs/watcher/test/file_watcher/native_test.dart b/pkgs/watcher/test/file_watcher/native_test.dart index 0d4ad6394c..30f3194327 100644 --- a/pkgs/watcher/test/file_watcher/native_test.dart +++ b/pkgs/watcher/test/file_watcher/native_test.dart @@ -9,14 +9,12 @@ import 'package:test/test.dart'; import 'package:watcher/src/file_watcher/native.dart'; import '../utils.dart'; -import 'shared.dart'; +import 'file_tests.dart'; +import 'startup_race_tests.dart'; void main() { watcherFactory = NativeFileWatcher.new; - setUp(() { - writeFile('file.txt'); - }); - - sharedTests(); + fileTests(isNative: true); + startupRaceTests(isNative: true); } diff --git a/pkgs/watcher/test/file_watcher/polling_test.dart b/pkgs/watcher/test/file_watcher/polling_test.dart index 861fcb222a..b8e41570d7 100644 --- a/pkgs/watcher/test/file_watcher/polling_test.dart +++ b/pkgs/watcher/test/file_watcher/polling_test.dart @@ -2,19 +2,16 @@ // 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 'package:test/test.dart'; import 'package:watcher/watcher.dart'; import '../utils.dart'; -import 'shared.dart'; +import 'file_tests.dart'; +import 'startup_race_tests.dart'; void main() { watcherFactory = (file) => PollingFileWatcher(file, pollingDelay: const Duration(milliseconds: 100)); - setUp(() { - writeFile('file.txt'); - }); - - sharedTests(); + fileTests(isNative: false); + startupRaceTests(isNative: false); } diff --git a/pkgs/watcher/test/file_watcher/startup_race_tests.dart b/pkgs/watcher/test/file_watcher/startup_race_tests.dart new file mode 100644 index 0000000000..59e4f8e7b9 --- /dev/null +++ b/pkgs/watcher/test/file_watcher/startup_race_tests.dart @@ -0,0 +1,39 @@ +// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file +// 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:test/test.dart'; + +import '../utils.dart'; + +/// Tests for a startup race that affects MacOS. +/// +/// As documented in `File.watch`, changes from shortly _before_ the `watch` +/// method is called might be reported on MacOS. They should be ignored. +void startupRaceTests({required bool isNative}) { + test('ignores events from before watch starts', () async { + // Write then immediately watch 100 times and count the events received. + var events = 0; + final futures = >[]; + for (var i = 0; i != 100; ++i) { + writeFile('file$i.txt'); + await startWatcher(path: 'file$i.txt'); + futures.add( + waitForEvent().then((event) { + if (event != null) ++events; + }), + ); + } + await Future.wait(futures); + + // TODO(davidmorgan): the MacOS watcher currently does get unwanted events, + // fix it. + if (isNative && Platform.isMacOS) { + expect(events, greaterThan(10)); + } else { + expect(events, 0); + } + }); +} diff --git a/pkgs/watcher/test/utils.dart b/pkgs/watcher/test/utils.dart index 7867b9fc29..12a277980e 100644 --- a/pkgs/watcher/test/utils.dart +++ b/pkgs/watcher/test/utils.dart @@ -63,9 +63,13 @@ Future startWatcher({String? path}) async { final normalized = p.normalize(p.relative(path, from: d.sandbox)); // Make sure we got a path in the sandbox. - assert(p.isRelative(normalized) && !normalized.startsWith('..'), - 'Path is not in the sandbox: $path not in ${d.sandbox}'); - + if (!p.isRelative(normalized) || normalized.startsWith('..')) { + // The polling watcher can poll during test teardown, signal using an + // exception that it will ignore. + throw FileSystemException( + 'Path is not in the sandbox: $path not in ${d.sandbox}', + ); + } var mtime = _mockFileModificationTimes[normalized]; return mtime != null ? DateTime.fromMillisecondsSinceEpoch(mtime) : null; }); @@ -174,6 +178,23 @@ Matcher isModifyEvent(String path) => isWatchEvent(ChangeType.MODIFY, path); /// [path]. Matcher isRemoveEvent(String path) => isWatchEvent(ChangeType.REMOVE, path); +/// Takes the first event emitted during [duration], or returns `null` if there +/// is none. +Future waitForEvent({ + Duration duration = const Duration(seconds: 1), +}) async { + final result = await _watcherEvents.peek + .then((e) => e) + .timeout(duration, onTimeout: () => null); + if (result != null) _watcherEvents.take(1).ignore(); + return result; +} + +/// Expects that no events are omitted for [duration]. +Future expectNoEvents({Duration duration = const Duration(seconds: 1)}) async { + expect(await waitForEvent(duration: duration), isNull); +} + /// Expects that the next event emitted will be for an add event for [path]. Future expectAddEvent(String path) => _expectOrCollect(isWatchEvent(ChangeType.ADD, path));