OSSCA Chromium

기여 목록 / 8275731

base: Split FilePathWatcherTest.WatchDirectory into smaller tests

BasetestIn Review

FilePathWatcherTest.WatchDirectory를 파일 생성·수정·삭제 후 생성 테스트로 나누어 테스트 분리에 관한 TODO를 해결했습니다.

문제 설명

기존 WatchDirectory는 파일 생성·수정·삭제·이후 생성을 하나의 테스트에서 순서대로 검사했습니다. 각 동작을 독립적으로 실행하고 확인할 수 있도록 분리했습니다.

해결 내용

  • CreatedFileInDirectory, ModifiedFileInDirectory, CreatedFileInDirectoryAfterDeletion의 세 테스트로 나눴습니다.
  • 수정과 삭제 후 생성 테스트는 디렉터리와 초기 파일을 감시 등록 전에 준비하도록 했습니다.
  • Android에서는 준비 과정의 파일 시스템 이벤트가 감시에 섞이지 않도록, 감시 등록 전에 SpinEventLoopForABit()를 호출했습니다.
  • 삭제 검증은 삭제 후 생성 테스트에 포함하고, 기존 플랫폼별 이벤트 기대값을 유지했습니다.

전체 코드 diff는 다음과 같습니다.

diff --git a/base/files/file_path_watcher_unittest.cc b/base/files/file_path_watcher_unittest.cc
index 14fbca4..cf4176f3 100644
--- a/base/files/file_path_watcher_unittest.cc
+++ b/base/files/file_path_watcher_unittest.cc
@@ -891,12 +891,11 @@
   delegate.RunUntilEventsMatch(event_expecter);
 }

-// TODO(crbug.com/40263777): Split into smaller tests.
-TEST_F(FilePathWatcherTest, WatchDirectory) {
-  FilePathWatcher watcher;
+TEST_F(FilePathWatcherTest, CreatedFileInDirectory) {
   FilePath dir(temp_dir_.GetPath().AppendASCII("dir"));
   FilePath file1(dir.AppendASCII("file1"));
-  FilePath file2(dir.AppendASCII("file2"));
+
+  FilePathWatcher watcher;
   TestDelegate delegate;
   AccumulatingEventExpecter event_expecter;
   ASSERT_TRUE(SetupWatch(dir, &watcher, &delegate,
@@ -913,6 +912,27 @@
     event_expecter.AddExpectedEventForPath(dir);
   }
   delegate.RunUntilEventsMatch(event_expecter);
+}
+
+TEST_F(FilePathWatcherTest, ModifiedFileInDirectory) {
+  FilePath dir(temp_dir_.GetPath().AppendASCII("dir"));
+  FilePath file1(dir.AppendASCII("file1"));
+  ASSERT_TRUE(CreateDirectory(dir));
+  ASSERT_TRUE(WriteFile(file1, "content"));
+#if BUILDFLAG(IS_ANDROID)
+  // TODO(crbug.com/40286767): There appears to be a race condition
+  // between setting up the inotify watch and the processing of the file system
+  // notifications created while setting up the file system for this test. Spin
+  // the event loop to ensure that the events have been processed by the time
+  // the inotify watch has been set up.
+  SpinEventLoopForABit();
+#endif  // BUILDFLAG(IS_ANDROID)
+
+  FilePathWatcher watcher;
+  TestDelegate delegate;
+  AccumulatingEventExpecter event_expecter;
+  ASSERT_TRUE(SetupWatch(dir, &watcher, &delegate,
+                         FilePathWatcher::Type::kNonRecursive));

   ASSERT_TRUE(WriteFile(file1, "content v2"));
 #if BUILDFLAG(IS_APPLE)
@@ -932,6 +952,28 @@
 #endif
   delegate.RunUntilEventsMatch(event_expecter);
 #endif  // BUILDFLAG(IS_APPLE)
+}
+
+TEST_F(FilePathWatcherTest, CreatedFileInDirectoryAfterDeletion) {
+  FilePath dir(temp_dir_.GetPath().AppendASCII("dir"));
+  FilePath file1(dir.AppendASCII("file1"));
+  FilePath file2(dir.AppendASCII("file2"));
+  ASSERT_TRUE(CreateDirectory(dir));
+  ASSERT_TRUE(WriteFile(file1, "content"));
+#if BUILDFLAG(IS_ANDROID)
+  // TODO(crbug.com/40286767): There appears to be a race condition
+  // between setting up the inotify watch and the processing of the file system
+  // notifications created while setting up the file system for this test. Spin
+  // the event loop to ensure that the events have been processed by the time
+  // the inotify watch has been set up.
+  SpinEventLoopForABit();
+#endif  // BUILDFLAG(IS_ANDROID)
+
+  FilePathWatcher watcher;
+  TestDelegate delegate;
+  AccumulatingEventExpecter event_expecter;
+  ASSERT_TRUE(SetupWatch(dir, &watcher, &delegate,
+                         FilePathWatcher::Type::kNonRecursive));

   ASSERT_TRUE(DeleteFile(file1));
   VLOG(1) << "Waiting for file1 deletion";

테스트 방법

CL에 기록한 검증 명령은 다음과 같습니다.

base_unittests --gtest_filter=FilePathWatcherTest.*

Gerrit에서 patchset 3의 CQ 실행 통과를 확인했습니다. 현재 리뷰 중입니다.

배운 점

테스트를 분리할 때는 중복된 검증을 줄이고 준비 과정과 검증 대상을 구분해야 합니다. 비동기 파일 감시는 준비 과정에서 발생한 이벤트가 이후 감시에 섞일 수 있다는 점도 고려해야 합니다.

참고 자료