From 5b5987c17c91e7f27f83d4ac16968c37925d5d25 Mon Sep 17 00:00:00 2001 From: Hinrich Mahler Date: Wed, 4 Mar 2020 08:09:19 +0100 Subject: [PATCH 1/5] Use TemporaryDirectory in test_persistence --- tests/test_persistence.py | 57 ++++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 25 deletions(-) diff --git a/tests/test_persistence.py b/tests/test_persistence.py index 1927e154932..d4b4be4b371 100644 --- a/tests/test_persistence.py +++ b/tests/test_persistence.py @@ -18,6 +18,8 @@ # along with this program. If not, see [http://www.gnu.org/licenses/]. import signal import sys +import shutil +import tempfile from telegram.utils.helpers import encode_conversations_to_json @@ -37,6 +39,35 @@ PicklePersistence, CommandHandler, DictPersistence, TypeHandler +def setup_module(module): + # Switch to a temporary directory so we don't have to worry about cleaning up files + module.orig_dir = os.getcwd() + temp_dir = tempfile.TemporaryDirectory() + os.chdir(temp_dir.name) + + +def teardown_module(module): + # Go back to original directory + os.chdir(module.orig_dir) + + +@pytest.fixture(autouse=True) +def remove_files(): + # Remove all files from disk after every test + # Only removes files in the current, temporary directory + # That way we don't need to keep track of what files currently exist + yield + for filename in os.listdir(os.getcwd()): + file_path = os.path.join(os.getcwd(), filename) + try: + if os.path.isfile(file_path) or os.path.islink(file_path): + os.unlink(file_path) + elif os.path.isdir(file_path): + shutil.rmtree(file_path) + except Exception as e: + print('Failed to delete %s. Reason: %s' % (file_path, e)) + + @pytest.fixture(scope="function") def base_persistence(): return BasePersistence(store_chat_data=True, store_user_data=True, store_bot_data=True) @@ -310,9 +341,6 @@ def bad_pickle_files(): with open(name, 'w') as f: f.write('(())') yield True - for name in ['pickletest_user_data', 'pickletest_chat_data', 'pickletest_bot_data', - 'pickletest_conversations', 'pickletest']: - os.remove(name) @pytest.fixture(scope='function') @@ -330,9 +358,6 @@ def good_pickle_files(user_data, chat_data, bot_data, conversations): with open('pickletest', 'wb') as f: pickle.dump(data, f) yield True - for name in ['pickletest_user_data', 'pickletest_chat_data', 'pickletest_bot_data', - 'pickletest_conversations', 'pickletest']: - os.remove(name) @pytest.fixture(scope='function') @@ -347,9 +372,6 @@ def pickle_files_wo_bot_data(user_data, chat_data, conversations): with open('pickletest', 'wb') as f: pickle.dump(data, f) yield True - for name in ['pickletest_user_data', 'pickletest_chat_data', - 'pickletest_conversations', 'pickletest']: - os.remove(name) @pytest.fixture(scope='function') @@ -362,6 +384,7 @@ def update(bot): class TestPickelPersistence(object): def test_no_files_present_multi_file(self, pickle_persistence): + print(os.getcwd()) assert pickle_persistence.get_user_data() == defaultdict(dict) assert pickle_persistence.get_user_data() == defaultdict(dict) assert pickle_persistence.get_chat_data() == defaultdict(dict) @@ -776,9 +799,6 @@ def test_flush_on_stop(self, bot, update, pickle_persistence): assert pickle_persistence_2.get_bot_data()['test'] == 'Working3!' def test_flush_on_stop_only_bot(self, bot, update, pickle_persistence_only_bot): - os.remove('pickletest_user_data') - os.remove('pickletest_chat_data') - os.remove('pickletest_bot_data') u = Updater(bot=bot, persistence=pickle_persistence_only_bot) dp = u.dispatcher u.running = True @@ -800,7 +820,6 @@ def test_flush_on_stop_only_bot(self, bot, update, pickle_persistence_only_bot): assert pickle_persistence_2.get_bot_data()['my_test3'] == 'Working3!' def test_flush_on_stop_only_chat(self, bot, update, pickle_persistence_only_chat): - os.remove('pickletest_bot_data') u = Updater(bot=bot, persistence=pickle_persistence_only_chat) dp = u.dispatcher u.running = True @@ -821,7 +840,6 @@ def test_flush_on_stop_only_chat(self, bot, update, pickle_persistence_only_chat assert pickle_persistence_2.get_bot_data() == {} def test_flush_on_stop_only_user(self, bot, update, pickle_persistence_only_user): - os.remove('pickletest_chat_data') u = Updater(bot=bot, persistence=pickle_persistence_only_user) dp = u.dispatcher u.running = True @@ -923,17 +941,6 @@ def next2(update, context): assert nested_ch.conversations[nested_ch._get_key(update)] == 1 assert nested_ch.conversations == pickle_persistence.conversations['name3'] - @classmethod - def teardown_class(cls): - try: - for name in ['pickletest_user_data', 'pickletest_chat_data', - 'pickletest_bot_data', - 'pickletest_conversations', - 'pickletest']: - os.remove(name) - except Exception: - pass - @pytest.fixture(scope='function') def user_data_json(user_data): From e07ab92c3de2ea353c5ee45ff70a5d20b856dd94 Mon Sep 17 00:00:00 2001 From: Hinrich Mahler Date: Wed, 4 Mar 2020 08:10:54 +0100 Subject: [PATCH 2/5] remove debug print --- tests/test_persistence.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_persistence.py b/tests/test_persistence.py index d4b4be4b371..90e59be2fba 100644 --- a/tests/test_persistence.py +++ b/tests/test_persistence.py @@ -384,7 +384,6 @@ def update(bot): class TestPickelPersistence(object): def test_no_files_present_multi_file(self, pickle_persistence): - print(os.getcwd()) assert pickle_persistence.get_user_data() == defaultdict(dict) assert pickle_persistence.get_user_data() == defaultdict(dict) assert pickle_persistence.get_chat_data() == defaultdict(dict) From 79b962eccc2305d9e57d24bd5ad8e4e5c9d4d31b Mon Sep 17 00:00:00 2001 From: Hinrich Mahler Date: Wed, 4 Mar 2020 19:47:10 +0100 Subject: [PATCH 3/5] just change the dir on every test --- tests/test_persistence.py | 35 ++++++++--------------------------- 1 file changed, 8 insertions(+), 27 deletions(-) diff --git a/tests/test_persistence.py b/tests/test_persistence.py index 90e59be2fba..292513137da 100644 --- a/tests/test_persistence.py +++ b/tests/test_persistence.py @@ -18,7 +18,6 @@ # along with this program. If not, see [http://www.gnu.org/licenses/]. import signal import sys -import shutil import tempfile from telegram.utils.helpers import encode_conversations_to_json @@ -39,33 +38,15 @@ PicklePersistence, CommandHandler, DictPersistence, TypeHandler -def setup_module(module): - # Switch to a temporary directory so we don't have to worry about cleaning up files - module.orig_dir = os.getcwd() - temp_dir = tempfile.TemporaryDirectory() - os.chdir(temp_dir.name) - - -def teardown_module(module): - # Go back to original directory - os.chdir(module.orig_dir) - - @pytest.fixture(autouse=True) -def remove_files(): - # Remove all files from disk after every test - # Only removes files in the current, temporary directory - # That way we don't need to keep track of what files currently exist - yield - for filename in os.listdir(os.getcwd()): - file_path = os.path.join(os.getcwd(), filename) - try: - if os.path.isfile(file_path) or os.path.islink(file_path): - os.unlink(file_path) - elif os.path.isdir(file_path): - shutil.rmtree(file_path) - except Exception as e: - print('Failed to delete %s. Reason: %s' % (file_path, e)) +def change_directory(): + orig_dir = os.getcwd() + with tempfile.TemporaryDirectory() as tmpdirname: + # Switch to a temporary directory so we don't have to worry about cleaning up files + os.chdir(tmpdirname) + yield + # Go back to original directory + os.chdir(orig_dir) @pytest.fixture(scope="function") From cf4bee2612a0d389d7379e42494068fd86fa2d10 Mon Sep 17 00:00:00 2001 From: Hinrich Mahler Date: Wed, 4 Mar 2020 22:30:49 +0100 Subject: [PATCH 4/5] Use pytests built in tmp_path --- tests/test_persistence.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/tests/test_persistence.py b/tests/test_persistence.py index 292513137da..435d8a86c5d 100644 --- a/tests/test_persistence.py +++ b/tests/test_persistence.py @@ -18,7 +18,6 @@ # along with this program. If not, see [http://www.gnu.org/licenses/]. import signal import sys -import tempfile from telegram.utils.helpers import encode_conversations_to_json @@ -39,14 +38,13 @@ @pytest.fixture(autouse=True) -def change_directory(): +def change_directory(tmp_path): orig_dir = os.getcwd() - with tempfile.TemporaryDirectory() as tmpdirname: - # Switch to a temporary directory so we don't have to worry about cleaning up files - os.chdir(tmpdirname) - yield - # Go back to original directory - os.chdir(orig_dir) + # Switch to a temporary directory so we don't have to worry about cleaning up files + os.chdir(tmp_path) + yield + # Go back to original directory + os.chdir(orig_dir) @pytest.fixture(scope="function") From 623352ed0eaa7c1fe8a09333fcbd5544b5171fa0 Mon Sep 17 00:00:00 2001 From: Hinrich Mahler Date: Wed, 4 Mar 2020 22:39:52 +0100 Subject: [PATCH 5/5] Fix for py3.5 --- tests/test_persistence.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_persistence.py b/tests/test_persistence.py index 435d8a86c5d..9e9b2665c68 100644 --- a/tests/test_persistence.py +++ b/tests/test_persistence.py @@ -41,7 +41,8 @@ def change_directory(tmp_path): orig_dir = os.getcwd() # Switch to a temporary directory so we don't have to worry about cleaning up files - os.chdir(tmp_path) + # (str() for py<3.6) + os.chdir(str(tmp_path)) yield # Go back to original directory os.chdir(orig_dir)