From d50b916394492960e10b0e3ccedaaa2c879645fa Mon Sep 17 00:00:00 2001 From: Chris Dinh Date: Fri, 6 Dec 2024 16:08:39 -0800 Subject: [PATCH] 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 | 182 +++++++++++++++++++++---------- 1 file changed, 125 insertions(+), 57 deletions(-) diff --git a/eden/integration/changes_test.py b/eden/integration/changes_test.py index 8ba929e604419..95c16cfd148ec 100644 --- a/eden/integration/changes_test.py +++ b/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,39 +202,50 @@ 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" ), @@ -191,16 +253,7 @@ def test_modify_file(self): 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")