From e75089bae209d1b9ca72903c0d65530b02f67fdf Mon Sep 17 00:00:00 2001 From: Drummond Ogilvie Date: Fri, 16 Sep 2022 15:46:09 +0100 Subject: [PATCH] Add and fix tests for init-hook based plugin load (#7475) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add and fix tests for init-hook based plugin load This changes 4 existing tests, due to a misunderstanding of the author (past me) in a couple of the details of those tests. Specifically, we now copy a single python file to use as our plugin, and make sure to correctly check for the name as given in the checker classes. We also make sure not to accidentally load the old copy of the plugin, which apparently sits in a directory that is already on the system path. There is also a single new test, which covers the cases of loading a plugin with ``init-hook`` magic, both specified in an rc file, but in different orders. This should be sufficient to close the issue around #7264. Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com> --- doc/whatsnew/fragments/7264.internal | 9 ++ tests/lint/unittest_lint.py | 174 ++++++++++++++++++++------- 2 files changed, 142 insertions(+), 41 deletions(-) create mode 100644 doc/whatsnew/fragments/7264.internal diff --git a/doc/whatsnew/fragments/7264.internal b/doc/whatsnew/fragments/7264.internal new file mode 100644 index 0000000000..2bd3337bd7 --- /dev/null +++ b/doc/whatsnew/fragments/7264.internal @@ -0,0 +1,9 @@ +Add and fix regression tests for plugin loading. + +This shores up the tests that cover the loading of custom plugins as affected +by any changes made to the ``sys.path`` during execution of an ``init-hook``. +Given the existing contract of allowing plugins to be loaded by fiddling with +the path in this way, this is now the last bit of work needed to close Github +issue #7264. + +Closes #7264 diff --git a/tests/lint/unittest_lint.py b/tests/lint/unittest_lint.py index 80532dea71..25df951df4 100644 --- a/tests/lint/unittest_lint.py +++ b/tests/lint/unittest_lint.py @@ -19,7 +19,7 @@ from os import chdir, getcwd from os.path import abspath, dirname, join, sep from pathlib import Path -from shutil import copytree, rmtree +from shutil import copy, rmtree import platformdirs import pytest @@ -534,7 +534,9 @@ def test_load_plugin_path_manipulation_case_6() -> None: the config file has run. This is not supported, and was previously a silent failure. This test ensures a ``bad-plugin-value`` message is emitted. """ - dummy_plugin_path = abspath(join(REGRTEST_DATA_DIR, "dummy_plugin")) + dummy_plugin_path = abspath( + join(REGRTEST_DATA_DIR, "dummy_plugin", "dummy_plugin.py") + ) with fake_home() as home_path: # construct a basic rc file that just modifies the path pylintrc_file = join(home_path, "pylintrc") @@ -547,7 +549,7 @@ def test_load_plugin_path_manipulation_case_6() -> None: ] ) - copytree(dummy_plugin_path, join(home_path, "copy_dummy")) + copy(dummy_plugin_path, join(home_path, "copy_dummy.py")) # To confirm we won't load this module _without_ the init hook running. assert home_path not in sys.path @@ -566,7 +568,7 @@ def test_load_plugin_path_manipulation_case_6() -> None: assert run._rcfile == pylintrc_file assert home_path in sys.path # The module should not be loaded - assert not any(ch.name == "copy_dummy" for ch in run.linter.get_checkers()) + assert not any(ch.name == "dummy_plugin" for ch in run.linter.get_checkers()) # There should be a bad-plugin-message for this module assert len(run.linter.reporter.messages) == 1 @@ -603,7 +605,9 @@ def test_load_plugin_path_manipulation_case_3() -> None: the config file has run. This is not supported, and was previously a silent failure. This test ensures a ``bad-plugin-value`` message is emitted. """ - dummy_plugin_path = abspath(join(REGRTEST_DATA_DIR, "dummy_plugin")) + dummy_plugin_path = abspath( + join(REGRTEST_DATA_DIR, "dummy_plugin", "dummy_plugin.py") + ) with fake_home() as home_path: # construct a basic rc file that just modifies the path pylintrc_file = join(home_path, "pylintrc") @@ -615,7 +619,7 @@ def test_load_plugin_path_manipulation_case_3() -> None: ] ) - copytree(dummy_plugin_path, join(home_path, "copy_dummy")) + copy(dummy_plugin_path, join(home_path, "copy_dummy.py")) # To confirm we won't load this module _without_ the init hook running. assert home_path not in sys.path @@ -634,7 +638,7 @@ def test_load_plugin_path_manipulation_case_3() -> None: assert run._rcfile == pylintrc_file assert home_path in sys.path # The module should not be loaded - assert not any(ch.name == "copy_dummy" for ch in run.linter.get_checkers()) + assert not any(ch.name == "dummy_plugin" for ch in run.linter.get_checkers()) # There should be a bad-plugin-message for this module assert len(run.linter.reporter.messages) == 1 @@ -662,49 +666,137 @@ def test_load_plugin_path_manipulation_case_3() -> None: sys.path.remove(home_path) -def test_load_plugin_command_line_before_init_hook() -> None: - """Check that the order of 'load-plugins' and 'init-hook' doesn't affect execution.""" - regrtest_data_dir_abs = abspath(REGRTEST_DATA_DIR) +@pytest.mark.usefixtures("pop_pylintrc") +def test_load_plugin_pylintrc_order_independent() -> None: + """Test that the init-hook is called independent of the order in a config file. - run = Run( - [ - "--load-plugins", - "dummy_plugin", - "--init-hook", - f'import sys; sys.path.append("{regrtest_data_dir_abs}")', - join(REGRTEST_DATA_DIR, "empty.py"), - ], - exit=False, - ) - assert ( - len([ch.name for ch in run.linter.get_checkers() if ch.name == "dummy_plugin"]) - == 2 + We want to ensure that any path manipulation in init hook + that means a plugin can load (as per GitHub Issue #7264 Cases 4+7) + runs before the load call, regardless of the order of lines in the + pylintrc file. + """ + dummy_plugin_path = abspath( + join(REGRTEST_DATA_DIR, "dummy_plugin", "dummy_plugin.py") ) - # Necessary as the executed init-hook modifies sys.path - sys.path.remove(regrtest_data_dir_abs) + with fake_home() as home_path: + copy(dummy_plugin_path, join(home_path, "copy_dummy.py")) + # construct a basic rc file that just modifies the path + pylintrc_file_before = join(home_path, "pylintrc_before") + with open(pylintrc_file_before, "w", encoding="utf8") as out: + out.writelines( + [ + "[MASTER]\n", + f"init-hook=\"import sys; sys.path.append(r'{home_path}')\"\n", + "load-plugins=copy_dummy\n", + ] + ) + pylintrc_file_after = join(home_path, "pylintrc_after") + with open(pylintrc_file_after, "w", encoding="utf8") as out: + out.writelines( + [ + "[MASTER]\n", + "load-plugins=copy_dummy\n" + f"init-hook=\"import sys; sys.path.append(r'{home_path}')\"\n", + ] + ) + for rcfile in (pylintrc_file_before, pylintrc_file_after): + # To confirm we won't load this module _without_ the init hook running. + assert home_path not in sys.path + run = Run( + [ + "--rcfile", + rcfile, + join(REGRTEST_DATA_DIR, "empty.py"), + ], + exit=False, + ) + assert ( + len( + [ + ch.name + for ch in run.linter.get_checkers() + if ch.name == "dummy_plugin" + ] + ) + == 2 + ) + assert run._rcfile == rcfile + assert home_path in sys.path + # Necessary as the executed init-hook modifies sys.path + sys.path.remove(home_path) -def test_load_plugin_command_line_with_init_hook_command_line() -> None: - regrtest_data_dir_abs = abspath(REGRTEST_DATA_DIR) - run = Run( - [ - "--init-hook", - f'import sys; sys.path.append("{regrtest_data_dir_abs}")', - "--load-plugins", - "dummy_plugin", - join(REGRTEST_DATA_DIR, "empty.py"), - ], - exit=False, +def test_load_plugin_command_line_before_init_hook() -> None: + """Check that the order of 'load-plugins' and 'init-hook' doesn't affect execution.""" + dummy_plugin_path = abspath( + join(REGRTEST_DATA_DIR, "dummy_plugin", "dummy_plugin.py") ) - assert ( - len([ch.name for ch in run.linter.get_checkers() if ch.name == "dummy_plugin"]) - == 2 + + with fake_home() as home_path: + copy(dummy_plugin_path, join(home_path, "copy_dummy.py")) + # construct a basic rc file that just modifies the path + assert home_path not in sys.path + run = Run( + [ + "--load-plugins", + "copy_dummy", + "--init-hook", + f'import sys; sys.path.append(r"{home_path}")', + join(REGRTEST_DATA_DIR, "empty.py"), + ], + exit=False, + ) + assert home_path in sys.path + assert ( + len( + [ + ch.name + for ch in run.linter.get_checkers() + if ch.name == "dummy_plugin" + ] + ) + == 2 + ) + + # Necessary as the executed init-hook modifies sys.path + sys.path.remove(home_path) + + +def test_load_plugin_command_line_with_init_hook_command_line() -> None: + dummy_plugin_path = abspath( + join(REGRTEST_DATA_DIR, "dummy_plugin", "dummy_plugin.py") ) - # Necessary as the executed init-hook modifies sys.path - sys.path.remove(regrtest_data_dir_abs) + with fake_home() as home_path: + copy(dummy_plugin_path, join(home_path, "copy_dummy.py")) + # construct a basic rc file that just modifies the path + assert home_path not in sys.path + run = Run( + [ + "--init-hook", + f'import sys; sys.path.append(r"{home_path}")', + "--load-plugins", + "copy_dummy", + join(REGRTEST_DATA_DIR, "empty.py"), + ], + exit=False, + ) + assert ( + len( + [ + ch.name + for ch in run.linter.get_checkers() + if ch.name == "dummy_plugin" + ] + ) + == 2 + ) + assert home_path in sys.path + + # Necessary as the executed init-hook modifies sys.path + sys.path.remove(home_path) def test_load_plugin_config_file() -> None: