Refactor changessincev2 test to be os-aware

Summary:
# Context

We are introducing EdenFS notifications to support scalable and ergonomic file system notifications for EdenFS mounts.

# This Diff
Refactor the changes_test integration test to more cleanly separate windows and *NIx code pathways

# Technical Details
Extract most of of the Windows specific things into a subclass.
There's now 3 test classes, Common, Windows, and Nix. OS specific things will go into the respective categories

# Discussion Points

Reviewed By: jdelliot

Differential Revision: D66827581

fbshipit-source-id: 938c413f9034cde509a9e429819d31c8b759486d

eden/integration/changes_test.py

@@ -95,8 +95,7 @@ def buildSmallChange(
     return ChangeNotification()
 
 
-@testcase.eden_repo_test
-class ChangesTest(testcase.EdenRepoTest):
+class ChangesTestBase(testcase.EdenRepoTest):
     def populate_repo(self) -> None:
         # Create the initial repo. It requires at least 1 file and 1 commit
         self.repo.write_file("hello", "bonjour\n")
@@ -137,6 +136,58 @@ def getChangesSinceV2(self, position) -> ChangesSinceV2Result:
             )
         )
 
+    def repo_write_file(self, path, contents, mode=None, add=True) -> None:
+        self.eden_repo.write_file(path, contents, mode, add)
+
+    def setup_test_add_file(self) -> ChangesSinceV2Result:
+        position = self.client.getCurrentJournalPosition(self.mount_path_bytes)
+        self.repo_write_file("test_file", "", add=False)
+        return self.getChangesSinceV2(position=position)
+
+    def setup_test_rename_file(self) -> ChangesSinceV2Result:
+        self.repo_write_file("test_file", "", add=False)
+        position = self.client.getCurrentJournalPosition(self.mount_path_bytes)
+        self.rename("test_file", "best_file")
+        return self.getChangesSinceV2(position=position)
+
+
+class WindowsTestBase(ChangesTestBase):
+    SYNC_MAX: int = 1
+
+    def syncProjFS(self, position) -> None:
+        # Wait for eden to get the PrjFS notification
+        pollTime = 0.1
+        waitTime = 0
+        newPosition = self.client.getCurrentJournalPosition(self.mount_path_bytes)
+        while position == newPosition and waitTime < self.SYNC_MAX:
+            time.sleep(pollTime)
+            waitTime += pollTime
+            newPosition = self.client.getCurrentJournalPosition(self.mount_path_bytes)
+
+    def repo_write_file(self, path, contents, mode=None, add=True) -> None:
+        position = self.client.getCurrentJournalPosition(self.mount_path_bytes)
+        super().repo_write_file(path, contents, mode, add)
+        self.syncProjFS(position)
+
+    def rm(self, path) -> None:
+        position = self.client.getCurrentJournalPosition(self.mount_path_bytes)
+        super().rm(path)
+        self.syncProjFS(position)
+
+    def rename(self, from_path, to_path) -> None:
+        position = self.client.getCurrentJournalPosition(self.mount_path_bytes)
+        super().rename(from_path, to_path)
+        self.syncProjFS(position)
+
+
+if sys.platform == "win32":
+    testBase = WindowsTestBase
+else:
+    testBase = ChangesTestBase
+
+
+@testcase.eden_repo_test
+class ChangesTestCommon(testBase):
     def test_wrong_mount_generation(self):
         # The input mount generation should equal the current mount generation
         oldPosition = self.client.getCurrentJournalPosition(self.mount_path_bytes)
@@ -151,56 +202,58 @@ def test_wrong_mount_generation(self):
             LostChangesReason.EDENFS_REMOUNTED,
         )
 
-    def test_add_file(self):
+    def test_modify_file(self):
+        self.repo_write_file("test_file", "", add=False)
         position = self.client.getCurrentJournalPosition(self.mount_path_bytes)
-        self.eden_repo.write_file("test_file", "", add=False)
-        if sys.platform == "win32":
-            # Wait for eden to get the PrjFS notification
-            time.sleep(1)
+        self.repo_write_file("test_file", "contents", add=False)
         changes = self.getChangesSinceV2(position=position)
-        # When adding a file, it is technically written to so there's an additional modified operation
         expected_changes = [
-            buildSmallChange(
-                SmallChangeNotification.ADDED, Dtype.REGULAR, path=b"test_file"
-            ),
             buildSmallChange(
                 SmallChangeNotification.MODIFIED, Dtype.REGULAR, path=b"test_file"
             ),
         ]
-        if sys.platform == "win32":
-            # PrjFS notifications only shows up as added, so remove the modified
-            expected_changes.pop()
         self.assertTrue(self.check_changes(changes.changes, expected_changes))
 
-    def test_modify_file(self):
-        self.eden_repo.write_file("test_file", "", add=False)
-        if sys.platform == "win32":
-            # Wait for eden to get the PrjFS notification
-            time.sleep(1)
+    def test_remove_file(self):
+        self.repo_write_file("test_file", "", add=False)
         position = self.client.getCurrentJournalPosition(self.mount_path_bytes)
-        self.eden_repo.write_file("test_file", "contents", add=False)
-        if sys.platform == "win32":
-            # Wait for eden to get the PrjFS notification
-            time.sleep(1)
+        self.rm("test_file")
         changes = self.getChangesSinceV2(position=position)
         expected_changes = [
+            buildSmallChange(
+                SmallChangeNotification.REMOVED,
+                Dtype.REGULAR,
+                path=b"test_file",
+            ),
+        ]
+        self.assertTrue(self.check_changes(changes.changes, expected_changes))
+
+
+# The following tests have different results based on platform
+
+
+@testcase.eden_repo_test
+class ChangesTestNix(ChangesTestBase):
+    def setUp(self) -> None:
+        if sys.platform == "win32":
+            self.skipTest("Non-Windows test")
+        return super().setUp()
+
+    def test_add_file(self):
+        # When adding a file, it is technically written to so there's an additional modified operation
+        changes = self.setup_test_add_file()
+        expected_changes = [
+            buildSmallChange(
+                SmallChangeNotification.ADDED, Dtype.REGULAR, path=b"test_file"
+            ),
             buildSmallChange(
                 SmallChangeNotification.MODIFIED, Dtype.REGULAR, path=b"test_file"
             ),
         ]
         self.assertTrue(self.check_changes(changes.changes, expected_changes))
 
     def test_rename_file(self):
-        self.eden_repo.write_file("test_file", "", add=False)
-        if sys.platform == "win32":
-            # Wait for eden to get the PrjFS notification
-            time.sleep(1)
-        position = self.client.getCurrentJournalPosition(self.mount_path_bytes)
-        self.rename("test_file", "best_file")
-        if sys.platform == "win32":
-            # Wait for eden to get the PrjFS notification
-            time.sleep(1)
-        changes = self.getChangesSinceV2(position=position)
+        changes = self.setup_test_rename_file()
         expected_changes = [
             buildSmallChange(
                 SmallChangeNotification.RENAMED,
@@ -209,19 +262,8 @@ def test_rename_file(self):
                 to_path=b"best_file",
             ),
         ]
-        if sys.platform == "win32":
-            # No renames on windows, the files are added/removed
-            expected_changes = [
-                buildSmallChange(
-                    SmallChangeNotification.REMOVED, Dtype.REGULAR, path=b"test_file"
-                ),
-                buildSmallChange(
-                    SmallChangeNotification.ADDED, Dtype.REGULAR, path=b"best_file"
-                ),
-            ]
         self.assertTrue(self.check_changes(changes.changes, expected_changes))
 
-    @unittest.skipIf(sys.platform == "win32", "PrjFS does not support replace")
     def test_replace_file(self):
         self.eden_repo.write_file("test_file", "test_contents", add=False)
         self.eden_repo.write_file("gone_file", "replaced_contents", add=False)
@@ -238,22 +280,48 @@ def test_replace_file(self):
         ]
         self.assertTrue(self.check_changes(changes.changes, expected_changes))
 
-    def test_remove_file(self):
-        self.eden_repo.write_file("test_file", "", add=False)
-        if sys.platform == "win32":
-            # Wait for eden to get the PrjFS notification
-            time.sleep(1)
-        position = self.client.getCurrentJournalPosition(self.mount_path_bytes)
-        self.rm("test_file")
-        if sys.platform == "win32":
-            # Wait for eden to get the PrjFS notification
-            time.sleep(1)
-        changes = self.getChangesSinceV2(position=position)
+
+@testcase.eden_repo_test
+class ChangesTestWin(WindowsTestBase):
+    def setUp(self) -> None:
+        if sys.platform != "win32":
+            self.skipTest("Windows only test")
+        return super().setUp()
+
+    def test_add_file(self):
+        # In windows, the file is created and then modified in projfs, then eden gets
+        # a single ADDED notification
+        changes = self.setup_test_add_file()
         expected_changes = [
             buildSmallChange(
-                SmallChangeNotification.REMOVED,
+                SmallChangeNotification.ADDED, Dtype.REGULAR, path=b"test_file"
+            )
+        ]
+        self.assertTrue(self.check_changes(changes.changes, expected_changes))
+
+    def test_rename_file(self):
+        changes = self.setup_test_rename_file()
+        expected_changes = [
+            buildSmallChange(
+                SmallChangeNotification.RENAMED,
                 Dtype.REGULAR,
-                path=b"test_file",
+                from_path=b"test_file",
+                to_path=b"best_file",
+            ),
+        ]
+        expected_changes = [
+            buildSmallChange(
+                SmallChangeNotification.REMOVED, Dtype.REGULAR, path=b"test_file"
+            ),
+            buildSmallChange(
+                SmallChangeNotification.ADDED, Dtype.REGULAR, path=b"best_file"
             ),
         ]
         self.assertTrue(self.check_changes(changes.changes, expected_changes))
+
+    # Files cannot be replaced in windows
+    def test_replace_file(self):
+        self.repo_write_file("test_file", "test_contents", add=False)
+        self.repo_write_file("gone_file", "replaced_contents", add=False)
+        with self.assertRaises(FileExistsError):
+            self.rename("test_file", "gone_file")