Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1)

Issue 122573003: Don't test for file differences in the polling watcher. (Closed)

Created:
6 years, 11 months ago by Bob Nystrom
Modified:
6 years, 11 months ago
Reviewers:
nweiz
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Don't test for file differences in the polling watcher. The non-polling forms don't do this, so this makes the polling style behave the same. It also has two important benefits: 1. On Windows, this avoids opening the files being watched. Since opening a file prevents it from being deleted on Windows, this causes the watcher to interfere with the files and leads to some flaky tests and buggy user behavior. 2. It saves some time and memory since we don't need to store the SHA1 for every file being watched. BUG=http://dartbug.com/13026, http://dartbug.com/15431 R=nweiz@google.com Committed: https://code.google.com/p/dart/source/detail?r=31574

Patch Set 1 #

Patch Set 2 : Unflake tests. #

Total comments: 2

Patch Set 3 : Revise shared test. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+33 lines, -99 lines) Patch
M pkg/watcher/lib/src/directory_watcher/polling.dart View 6 chunks +14 lines, -55 lines 0 comments Download
M pkg/watcher/pubspec.yaml View 1 chunk +3 lines, -5 lines 0 comments Download
M pkg/watcher/test/directory_watcher/linux_test.dart View 1 2 1 chunk +0 lines, -10 lines 0 comments Download
M pkg/watcher/test/directory_watcher/mac_os_test.dart View 1 2 1 chunk +0 lines, -10 lines 0 comments Download
M pkg/watcher/test/directory_watcher/polling_test.dart View 1 chunk +0 lines, -9 lines 0 comments Download
M pkg/watcher/test/directory_watcher/shared.dart View 1 2 1 chunk +10 lines, -0 lines 0 comments Download
M sdk/lib/_internal/pub/pub.status View 1 2 1 chunk +0 lines, -2 lines 0 comments Download
M sdk/lib/_internal/pub/test/serve/missing_file_test.dart View 1 3 chunks +6 lines, -8 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
Bob Nystrom
6 years, 11 months ago (2013-12-31 01:08:58 UTC) #1
nweiz
lgtm https://codereview.chromium.org/122573003/diff/40001/pkg/watcher/test/directory_watcher/polling_test.dart File pkg/watcher/test/directory_watcher/polling_test.dart (left): https://codereview.chromium.org/122573003/diff/40001/pkg/watcher/test/directory_watcher/polling_test.dart#oldcode29 pkg/watcher/test/directory_watcher/polling_test.dart:29: }); There's a variant of this test in ...
6 years, 11 months ago (2014-01-06 23:51:52 UTC) #2
Bob Nystrom
https://codereview.chromium.org/122573003/diff/40001/pkg/watcher/test/directory_watcher/polling_test.dart File pkg/watcher/test/directory_watcher/polling_test.dart (left): https://codereview.chromium.org/122573003/diff/40001/pkg/watcher/test/directory_watcher/polling_test.dart#oldcode29 pkg/watcher/test/directory_watcher/polling_test.dart:29: }); On 2014/01/06 23:51:52, nweiz wrote: > There's a ...
6 years, 11 months ago (2014-01-07 21:09:04 UTC) #3
Bob Nystrom
6 years, 11 months ago (2014-01-07 21:10:04 UTC) #4
Message was sent while issue was closed.
Committed patchset #3 manually as r31574 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698