From 6754bffa2b2df15a741008aa611c1bb0e8dff22b Mon Sep 17 00:00:00 2001 From: Tom Forbes Date: Mon, 29 Apr 2019 10:44:58 +0200 Subject: [PATCH] Fixed #30323 -- Fixed detecting changes by autoreloader when using StatReloader. --- django/utils/autoreload.py | 42 ++++++++---------- docs/releases/2.2.1.txt | 3 ++ tests/utils_tests/test_autoreload.py | 64 +++------------------------- 3 files changed, 26 insertions(+), 83 deletions(-) diff --git a/django/utils/autoreload.py b/django/utils/autoreload.py index dc7c9b2cea..958feddb59 100644 --- a/django/utils/autoreload.py +++ b/django/utils/autoreload.py @@ -324,41 +324,33 @@ class StatReloader(BaseReloader): SLEEP_TIME = 1 # Check for changes once per second. def tick(self): - state, previous_timestamp = {}, time.time() + mtimes = {} while True: - state.update(self.loop_files(state, previous_timestamp)) - previous_timestamp = time.time() + for filepath, mtime in self.snapshot_files(): + old_time = mtimes.get(filepath) + if old_time is None: + logger.debug('File %s first seen with mtime %s', filepath, mtime) + mtimes[filepath] = mtime + continue + elif mtime > old_time: + logger.debug('File %s previous mtime: %s, current mtime: %s', filepath, old_time, mtime) + self.notify_file_changed(filepath) + time.sleep(self.SLEEP_TIME) yield - def loop_files(self, previous_times, previous_timestamp): - updated_times = {} - for path, mtime in self.snapshot_files(): - previous_time = previous_times.get(path) - # If there are overlapping globs, a file may be iterated twice. - if path in updated_times: - continue - # A new file has been detected. This could happen due to it being - # imported at runtime and only being polled now, or because the - # file was just created. Compare the file's mtime to the - # previous_timestamp and send a notification if it was created - # since the last poll. - is_newly_created = previous_time is None and mtime > previous_timestamp - is_changed = previous_time is not None and previous_time != mtime - if is_newly_created or is_changed: - logger.debug('File %s. is_changed: %s, is_new: %s', path, is_changed, is_newly_created) - logger.debug('File %s previous mtime: %s, current mtime: %s', path, previous_time, mtime) - self.notify_file_changed(path) - updated_times[path] = mtime - return updated_times - def snapshot_files(self): + # watched_files may produce duplicate paths if globs overlap. + seen_files = set() for file in self.watched_files(): + if file in seen_files: + continue try: mtime = file.stat().st_mtime except OSError: # This is thrown when the file does not exist. continue + seen_files.add(file) yield file, mtime @classmethod @@ -564,7 +556,7 @@ def start_django(reloader, main_func, *args, **kwargs): ensure_echo_on() main_func = check_errors(main_func) - django_main_thread = threading.Thread(target=main_func, args=args, kwargs=kwargs) + django_main_thread = threading.Thread(target=main_func, args=args, kwargs=kwargs, name='django-main-thread') django_main_thread.setDaemon(True) django_main_thread.start() diff --git a/docs/releases/2.2.1.txt b/docs/releases/2.2.1.txt index 4a2d77aaf3..9f241b83e9 100644 --- a/docs/releases/2.2.1.txt +++ b/docs/releases/2.2.1.txt @@ -67,3 +67,6 @@ Bugfixes * Fixed a regression in Django 2.2 that caused a crash of :djadmin:`runserver` when URLConf modules raised exceptions (:ticket:`30323`). + +* Fixed a regression in Django 2.2 where changes were not reliably detected by + auto-reloader when using ``StatReloader`` (:ticket:`30323`). diff --git a/tests/utils_tests/test_autoreload.py b/tests/utils_tests/test_autoreload.py index c59cb23cc3..42bcd0a50b 100644 --- a/tests/utils_tests/test_autoreload.py +++ b/tests/utils_tests/test_autoreload.py @@ -255,7 +255,7 @@ class StartDjangoTests(SimpleTestCase): self.assertEqual(mocked_thread.call_count, 1) self.assertEqual( mocked_thread.call_args[1], - {'target': fake_main_func, 'args': (123,), 'kwargs': {'abc': 123}} + {'target': fake_main_func, 'args': (123,), 'kwargs': {'abc': 123}, 'name': 'django-main-thread'} ) self.assertSequenceEqual(fake_thread.setDaemon.call_args[0], [True]) self.assertTrue(fake_thread.start.called) @@ -376,26 +376,6 @@ class IntegrationTests: self.assertEqual(notify_mock.call_count, 1) self.assertCountEqual(notify_mock.call_args[0], [self.existing_file]) - @mock.patch('django.utils.autoreload.BaseReloader.notify_file_changed') - @mock.patch('django.utils.autoreload.iter_all_python_module_files', return_value=frozenset()) - def test_nonexistent_file(self, mocked_modules, notify_mock): - self.reloader.watch_file(self.nonexistent_file) - with self.tick_twice(): - self.ensure_file(self.nonexistent_file) - self.assertEqual(notify_mock.call_count, 1) - self.assertCountEqual(notify_mock.call_args[0], [self.nonexistent_file]) - - @mock.patch('django.utils.autoreload.BaseReloader.notify_file_changed') - @mock.patch('django.utils.autoreload.iter_all_python_module_files', return_value=frozenset()) - def test_nonexistent_file_in_non_existing_directory(self, mocked_modules, notify_mock): - non_existing_directory = self.tempdir / 'non_existing_dir' - nonexistent_file = non_existing_directory / 'test' - self.reloader.watch_file(nonexistent_file) - with self.tick_twice(): - self.ensure_file(nonexistent_file) - self.assertEqual(notify_mock.call_count, 1) - self.assertCountEqual(notify_mock.call_args[0], [nonexistent_file]) - @mock.patch('django.utils.autoreload.BaseReloader.notify_file_changed') @mock.patch('django.utils.autoreload.iter_all_python_module_files', return_value=frozenset()) def test_glob(self, mocked_modules, notify_mock): @@ -407,18 +387,6 @@ class IntegrationTests: self.assertEqual(notify_mock.call_count, 1) self.assertCountEqual(notify_mock.call_args[0], [self.existing_file]) - @mock.patch('django.utils.autoreload.BaseReloader.notify_file_changed') - @mock.patch('django.utils.autoreload.iter_all_python_module_files', return_value=frozenset()) - def test_glob_non_existing_directory(self, mocked_modules, notify_mock): - non_existing_directory = self.tempdir / 'does_not_exist' - nonexistent_file = non_existing_directory / 'test.py' - self.reloader.watch_dir(non_existing_directory, '*.py') - with self.tick_twice(): - self.ensure_file(nonexistent_file) - self.set_mtime(nonexistent_file, time.time()) - self.assertEqual(notify_mock.call_count, 1) - self.assertCountEqual(notify_mock.call_args[0], [nonexistent_file]) - @mock.patch('django.utils.autoreload.BaseReloader.notify_file_changed') @mock.patch('django.utils.autoreload.iter_all_python_module_files', return_value=frozenset()) def test_multiple_globs(self, mocked_modules, notify_mock): @@ -669,28 +637,8 @@ class StatReloaderTests(ReloaderTests, IntegrationTests): snapshot2 = dict(self.reloader.snapshot_files()) self.assertNotEqual(snapshot1[self.existing_file], snapshot2[self.existing_file]) - def test_does_not_fire_without_changes(self): - with mock.patch.object(self.reloader, 'watched_files', return_value=[self.existing_file]), \ - mock.patch.object(self.reloader, 'notify_file_changed') as notifier: - mtime = self.existing_file.stat().st_mtime - initial_snapshot = {self.existing_file: mtime} - second_snapshot = self.reloader.loop_files(initial_snapshot, time.time()) - self.assertEqual(second_snapshot, {}) - notifier.assert_not_called() - - def test_fires_when_created(self): - with mock.patch.object(self.reloader, 'watched_files', return_value=[self.nonexistent_file]), \ - mock.patch.object(self.reloader, 'notify_file_changed') as notifier: - self.nonexistent_file.touch() - mtime = self.nonexistent_file.stat().st_mtime - second_snapshot = self.reloader.loop_files({}, mtime - 1) - self.assertCountEqual(second_snapshot.keys(), [self.nonexistent_file]) - notifier.assert_called_once_with(self.nonexistent_file) - - def test_fires_with_changes(self): - with mock.patch.object(self.reloader, 'watched_files', return_value=[self.existing_file]), \ - mock.patch.object(self.reloader, 'notify_file_changed') as notifier: - initial_snapshot = {self.existing_file: 1} - second_snapshot = self.reloader.loop_files(initial_snapshot, time.time()) - notifier.assert_called_once_with(self.existing_file) - self.assertCountEqual(second_snapshot.keys(), [self.existing_file]) + def test_snapshot_files_with_duplicates(self): + with mock.patch.object(self.reloader, 'watched_files', return_value=[self.existing_file, self.existing_file]): + snapshot = list(self.reloader.snapshot_files()) + self.assertEqual(len(snapshot), 1) + self.assertEqual(snapshot[0][0], self.existing_file)