Skip to content

Commit b8cf15a

Browse files
davidmorgandavidmorgan
committed
Add tests for file watcher behavior with links.
Fix MacOS startup race.
1 parent ecd7dd5 commit b8cf15a

File tree

9 files changed

+289
-22
lines changed

9 files changed

+289
-22
lines changed

‎pkgs/watcher/CHANGELOG.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
## 1.1.4-wip
2+
3+
- Bug fix: with `FileWatcher` on MacOS, an incorrect modify event was sometimes
4+
reported if the file was created immediately before the watcher was created.
5+
Now, file creation is never reported as a modification. This makes the behavior on
6+
MacOS consistent with other platforms and with the polling watcher.
7+
18
## 1.1.3
29

310
- Improve handling of

‎pkgs/watcher/lib/src/file_watcher/native.dart‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,14 @@ class _NativeFileWatcher implements FileWatcher, ManuallyClosedWatcher {
5858
return;
5959
}
6060

61+
// There should not be any `create` events: the watch is on a file that
62+
// already exists. On MacOS `File.watch` docs say "changes that occur
63+
// shortly _before_ the `watch` method is called may ... appear", and this
64+
// can cause a `create` event to be received. Ignore it.
65+
if (batch.every((event) => event.type == FileSystemEvent.create)) {
66+
return;
67+
}
68+
6169
_eventsController.add(WatchEvent(ChangeType.MODIFY, path));
6270
}
6371

‎pkgs/watcher/pubspec.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
name: watcher
2-
version: 1.1.3
2+
version: 1.1.4-wip
33
description: >-
44
A file system watcher. It monitors changes to contents of directories and
55
sends notifications when files have been added, removed, or modified.

‎pkgs/watcher/test/file_watcher/shared.dart‎ renamed to ‎pkgs/watcher/test/file_watcher/file_tests.dart‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,14 @@ import 'package:test/test.dart';
66

77
import '../utils.dart';
88

9-
void sharedTests() {
9+
void fileTests() {
10+
setUp(() async {
11+
writeFile('file.txt');
12+
});
13+
1014
test("doesn't notify if the file isn't modified", () async {
1115
await startWatcher(path: 'file.txt');
12-
await pumpEventQueue();
13-
deleteFile('file.txt');
14-
await expectRemoveEvent('file.txt');
16+
await expectNoEvents();
1517
});
1618

1719
test('notifies when a file is modified', () async {

‎pkgs/watcher/test/file_watcher/link_tests.dart‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:test/test.dart';
6+
7+
import '../utils.dart';
8+
9+
void linkTests({required bool isNative}) {
10+
setUp(() async {
11+
writeFile('target.txt');
12+
writeLink(link: 'link.txt', target: 'target.txt');
13+
});
14+
15+
test("doesn't notify if nothing is modified", () async {
16+
await startWatcher(path: 'link.txt');
17+
await expectNoEvents();
18+
});
19+
20+
test('notifies when a link is overwritten with an identical file', () async {
21+
await startWatcher(path: 'link.txt');
22+
writeFile('link.txt');
23+
await expectModifyEvent('link.txt');
24+
});
25+
26+
test('notifies when a link is overwritten with a different file', () async {
27+
await startWatcher(path: 'link.txt');
28+
writeFile('link.txt', contents: 'modified');
29+
await expectModifyEvent('link.txt');
30+
});
31+
32+
test(
33+
'notifies when a link target is overwritten with an identical file',
34+
() async {
35+
await startWatcher(path: 'link.txt');
36+
writeFile('target.txt');
37+
38+
// TODO(davidmorgan): reconcile differences.
39+
if (isNative) {
40+
await expectModifyEvent('link.txt');
41+
} else {
42+
await expectNoEvents();
43+
}
44+
},
45+
);
46+
47+
test('notifies when a link target is modified', () async {
48+
await startWatcher(path: 'link.txt');
49+
writeFile('target.txt', contents: 'modified');
50+
51+
// TODO(davidmorgan): reconcile differences.
52+
if (isNative) {
53+
await expectModifyEvent('link.txt');
54+
} else {
55+
await expectNoEvents();
56+
}
57+
});
58+
59+
test('notifies when a link is removed', () async {
60+
await startWatcher(path: 'link.txt');
61+
deleteFile('link.txt');
62+
63+
// TODO(davidmorgan): reconcile differences.
64+
if (isNative) {
65+
await expectNoEvents();
66+
} else {
67+
await expectRemoveEvent('link.txt');
68+
}
69+
});
70+
71+
test('notifies when a link target is removed', () async {
72+
await startWatcher(path: 'link.txt');
73+
deleteFile('target.txt');
74+
await expectRemoveEvent('link.txt');
75+
});
76+
77+
test('notifies when a link target is modified multiple times', () async {
78+
await startWatcher(path: 'link.txt');
79+
80+
writeFile('target.txt', contents: 'modified');
81+
82+
// TODO(davidmorgan): reconcile differences.
83+
if (isNative) {
84+
await expectModifyEvent('link.txt');
85+
} else {
86+
await expectNoEvents();
87+
}
88+
89+
writeFile('target.txt', contents: 'modified again');
90+
91+
// TODO(davidmorgan): reconcile differences.
92+
if (isNative) {
93+
await expectModifyEvent('link.txt');
94+
} else {
95+
await expectNoEvents();
96+
}
97+
});
98+
99+
test('notifies when a link is moved away', () async {
100+
await startWatcher(path: 'link.txt');
101+
renameFile('link.txt', 'new.txt');
102+
103+
// TODO(davidmorgan): reconcile differences.
104+
if (isNative) {
105+
await expectNoEvents();
106+
} else {
107+
await expectRemoveEvent('link.txt');
108+
}
109+
});
110+
111+
test('notifies when a link target is moved away', () async {
112+
await startWatcher(path: 'link.txt');
113+
renameFile('target.txt', 'new.txt');
114+
await expectRemoveEvent('link.txt');
115+
});
116+
117+
test('notifies when an identical file is moved over the link', () async {
118+
await startWatcher(path: 'link.txt');
119+
writeFile('old.txt');
120+
renameFile('old.txt', 'link.txt');
121+
122+
// TODO(davidmorgan): reconcile differences.
123+
if (isNative) {
124+
await expectNoEvents();
125+
} else {
126+
await expectModifyEvent('link.txt');
127+
}
128+
});
129+
130+
test('notifies when an different file is moved over the link', () async {
131+
await startWatcher(path: 'link.txt');
132+
writeFile('old.txt', contents: 'modified');
133+
renameFile('old.txt', 'link.txt');
134+
135+
// TODO(davidmorgan): reconcile differences.
136+
if (isNative) {
137+
await expectNoEvents();
138+
} else {
139+
await expectModifyEvent('link.txt');
140+
}
141+
});
142+
143+
test('notifies when an identical file is moved over the target', () async {
144+
await startWatcher(path: 'link.txt');
145+
writeFile('old.txt');
146+
renameFile('old.txt', 'target.txt');
147+
148+
// TODO(davidmorgan): reconcile differences.
149+
if (isNative) {
150+
await expectModifyEvent('link.txt');
151+
} else {
152+
await expectNoEvents();
153+
}
154+
});
155+
156+
test('notifies when a different file is moved over the target', () async {
157+
await startWatcher(path: 'link.txt');
158+
writeFile('old.txt', contents: 'modified');
159+
renameFile('old.txt', 'target.txt');
160+
161+
// TODO(davidmorgan): reconcile differences.
162+
if (isNative) {
163+
await expectModifyEvent('link.txt');
164+
} else {
165+
await expectNoEvents();
166+
}
167+
});
168+
}

‎pkgs/watcher/test/file_watcher/native_test.dart‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,14 @@ import 'package:test/test.dart';
99
import 'package:watcher/src/file_watcher/native.dart';
1010

1111
import '../utils.dart';
12-
import 'shared.dart';
12+
import 'file_tests.dart';
13+
import 'link_tests.dart';
14+
import 'startup_race_tests.dart';
1315

1416
void main() {
1517
watcherFactory = NativeFileWatcher.new;
1618

17-
setUp(() {
18-
writeFile('file.txt');
19-
});
20-
21-
sharedTests();
19+
fileTests();
20+
linkTests(isNative: true);
21+
startupRaceTests();
2222
}

‎pkgs/watcher/test/file_watcher/polling_test.dart‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,20 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5-
import 'package:test/test.dart';
65
import 'package:watcher/watcher.dart';
76

87
import '../utils.dart';
9-
import 'shared.dart';
8+
import 'file_tests.dart';
9+
import 'link_tests.dart';
10+
import 'startup_race_tests.dart';
1011

1112
void main() {
12-
watcherFactory = (file) =>
13-
PollingFileWatcher(file, pollingDelay: const Duration(milliseconds: 100));
13+
watcherFactory = (file) => PollingFileWatcher(
14+
file,
15+
pollingDelay: const Duration(milliseconds: 100),
16+
);
1417

15-
setUp(() {
16-
writeFile('file.txt');
17-
});
18-
19-
sharedTests();
18+
fileTests();
19+
linkTests(isNative: false);
20+
startupRaceTests();
2021
}

‎pkgs/watcher/test/file_watcher/startup_race_tests.dart‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:test/test.dart';
6+
7+
import '../utils.dart';
8+
9+
/// Tests for a startup race that affects MacOS.
10+
///
11+
/// As documented in `File.watch`, changes from shortly _before_ the `watch`
12+
/// method is called might be reported on MacOS. They should be ignored.
13+
/// Runs on other platforms too, where no special handling should be needed.
14+
void startupRaceTests() {
15+
test('writing then immediately watching catches most events', () async {
16+
// Write then immediately watch 100 times and count the events received.
17+
var events = 0;
18+
final futures = <Future<void>>[];
19+
for (var i = 0; i != 100; ++i) {
20+
writeFile('file$i.txt');
21+
await startWatcher(path: 'file$i.txt');
22+
futures.add(
23+
waitForEvent().then((event) {
24+
if (event != null) ++events;
25+
}),
26+
);
27+
}
28+
await Future.wait(futures);
29+
expect(events, 0);
30+
});
31+
}

‎pkgs/watcher/test/utils.dart‎

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,13 @@ Future<void> startWatcher({String? path}) async {
6363
final normalized = p.normalize(p.relative(path, from: d.sandbox));
6464

6565
// Make sure we got a path in the sandbox.
66-
assert(p.isRelative(normalized) && !normalized.startsWith('..'),
67-
'Path is not in the sandbox: $path not in ${d.sandbox}');
66+
if (!p.isRelative(normalized) || normalized.startsWith('..')) {
67+
// The polling watcher can poll during test teardown, signal using an
68+
// exception that it will ignore.
69+
throw FileSystemException(
70+
'Path is not in the sandbox: $path not in ${d.sandbox}',
71+
);
72+
}
6873

6974
var mtime = _mockFileModificationTimes[normalized];
7075
return mtime != null ? DateTime.fromMillisecondsSinceEpoch(mtime) : null;
@@ -174,6 +179,23 @@ Matcher isModifyEvent(String path) => isWatchEvent(ChangeType.MODIFY, path);
174179
/// [path].
175180
Matcher isRemoveEvent(String path) => isWatchEvent(ChangeType.REMOVE, path);
176181

182+
/// Takes the first event omitted during [duration], or returns `null` if there
183+
/// is none.
184+
Future<WatchEvent?> waitForEvent({
185+
Duration duration = const Duration(seconds: 1),
186+
}) async {
187+
final result = await _watcherEvents.peek
188+
.then<WatchEvent?>((e) => e)
189+
.timeout(duration, onTimeout: () => null);
190+
if (result != null) _watcherEvents.take(1).ignore();
191+
return result;
192+
}
193+
194+
/// Expects that no events are omitted for [duration].
195+
Future expectNoEvents({Duration duration = const Duration(seconds: 1)}) async {
196+
expect(await waitForEvent(duration: duration), isNull);
197+
}
198+
177199
/// Expects that the next event emitted will be for an add event for [path].
178200
Future expectAddEvent(String path) =>
179201
_expectOrCollect(isWatchEvent(ChangeType.ADD, path));
@@ -225,6 +247,34 @@ void writeFile(String path, {String? contents, bool? updateModified}) {
225247
}
226248
}
227249

250+
/// Schedules writing a file in the sandbox at [link] pointing to [target].
251+
///
252+
/// If [updateModified] is `false`, the mock file modification time is not
253+
/// changed.
254+
void writeLink({
255+
required String link,
256+
required String target,
257+
bool? updateModified,
258+
}) {
259+
updateModified ??= true;
260+
261+
var fullPath = p.join(d.sandbox, link);
262+
263+
// Create any needed subdirectories.
264+
var dir = Directory(p.dirname(fullPath));
265+
if (!dir.existsSync()) {
266+
dir.createSync(recursive: true);
267+
}
268+
269+
Link(fullPath).createSync(target);
270+
271+
if (updateModified) {
272+
link = p.normalize(link);
273+
274+
_mockFileModificationTimes[link] = _nextTimestamp++;
275+
}
276+
}
277+
228278
/// Schedules deleting a file in the sandbox at [path].
229279
void deleteFile(String path) {
230280
File(p.join(d.sandbox, path)).deleteSync();

0 commit comments

Comments
 (0)