Skip to content

Commit d3f1c8d

Browse files
frank-emrichfacebook-github-bot
authored andcommitted
fix server crash on folder rename
Summary: This fixes a bug introduced in D76124936: If we run `Edenfs_watcher` on a folder that's not the Eden mount point, we called `glob_files` in the wrong way, causing Eden to throw an error and making the server exit. What's worse is that this bug was not discovered by the test (`test_folder_rename` run by `EdenfsWatcherNonMountPointRepoTests`) for this exact scenario. The test passed because it renames a folder containing files with type errors, and then runs `hh check` to verify that the error messages show the updated paths (to check that the server has indeed noticed that the files inside the folder were moved). This test passed because when the server crashed, it was restarted by the monitor, after which it would re-check everything and pick up the moved files this way. To avoid this problem in the future, I've added a check to all `Edenfs_watcher` tests to make sure that the server hasn't restarted. Differential Revision: D76582277 fbshipit-source-id: 1fec471ef466f108e22ee87f93f312c80ea8a69e
1 parent 9ff4933 commit d3f1c8d

File tree

1 file changed

+14
-0
lines changed

1 file changed

+14
-0
lines changed

hphp/hack/test/integration/test_edenfs_file_watcher.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,13 @@ def assertEdenFsWatcherInitialized(driver: CommonTestDriver) -> None:
107107
assertServerLogContains(driver, "[edenfs_watcher][init] finished init")
108108

109109

110+
def assertServerNotCrashed(driver: CommonTestDriver) -> None:
111+
monitor_log = driver.get_all_logs(driver.repo_dir).all_monitor_logs
112+
113+
driver.assertFalse("Exit_status.Edenfs_watcher_failed" in monitor_log)
114+
driver.assertFalse("writing crash log" in monitor_log)
115+
116+
110117
class EdenfsWatcherTestDriver(common_tests.CommonTestDriver):
111118
"""Driver compatible with CommonTestDriver, but creating an Eden-backed repo.
112119
@@ -206,6 +213,13 @@ def tearDown(self) -> None:
206213
# but core testing logic, but we need to get this into every test
207214
assertEdenFsWatcherInitialized(self)
208215

216+
# Similarly, another check: hh_server's resiliance is sometimes hiding bugs:
217+
# We may accidentally crash the Edenfs_watcher, which can take the server down.
218+
# But then the monitor restarts it and it re-checks the testing repo.
219+
# From the outside, that looks very similar to the server correctly
220+
# picking up a change we've made to the testing repo!
221+
assertServerNotCrashed(self)
222+
209223
# For hygiene, we (re-)mount our Eden repo for each individual test.
210224
# Note that we must stop the server before unmounting the Eden repo that it works on.
211225
# 3 retries is the value used in CommonTestDriver.tearDown

0 commit comments

Comments
 (0)