From 25d6780bf84c0d337f981ad92df26e8952ef9b06 Mon Sep 17 00:00:00 2001 From: Wayne Werner Date: Tue, 13 Dec 2022 13:15:23 -0600 Subject: [PATCH 1/3] Put back deprecation warnings Checked the https://docs.python.org/3/library/warnings.html#warnings.filterwarnings docs but there wasn't any obvious reason why our warnings disappear. This restores the DeprecationWarnings showing up on the command line. --- changelog/62185.fixed | 1 + salt/__init__.py | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 changelog/62185.fixed diff --git a/changelog/62185.fixed b/changelog/62185.fixed new file mode 100644 index 000000000000..e0329c8bf33b --- /dev/null +++ b/changelog/62185.fixed @@ -0,0 +1 @@ +Restored Salt's DeprecationWarnings diff --git a/salt/__init__.py b/salt/__init__.py index ec551abbc62a..7600f6236d71 100644 --- a/salt/__init__.py +++ b/salt/__init__.py @@ -58,7 +58,8 @@ def load_module(self, name): "", # No deprecation message match DeprecationWarning, # This filter is for DeprecationWarnings r"^(salt|salt\.(.*))$", # Match module(s) 'salt' and 'salt.' - append=True, + # Do *NOT* add append=True here - if we do, salt's DeprecationWarnings will + # never show up ) # Filter the backports package UserWarning about being re-imported From 9c896fedef794d4ff3172a1f761515a7b81f401a Mon Sep 17 00:00:00 2001 From: Wayne Werner Date: Mon, 19 Dec 2022 23:14:34 -0600 Subject: [PATCH 2/3] Add tests for deprecation warnings Rather than just rely on the test suite, we also add a deprecation warning to the test module, which will enable a simple method for ensuring that DeprecationWarnings are correctly emitted. --- changelog/63315.added | 1 + salt/modules/test.py | 25 +++++++++++++++++++ .../pytests/integration/modules/test_test.py | 8 ++++++ 3 files changed, 34 insertions(+) create mode 100644 changelog/63315.added create mode 100644 tests/pytests/integration/modules/test_test.py diff --git a/changelog/63315.added b/changelog/63315.added new file mode 100644 index 000000000000..950abc93277b --- /dev/null +++ b/changelog/63315.added @@ -0,0 +1 @@ +Added deprecation_warning test state for ensuring that deprecation warnings are correctly emitted. diff --git a/salt/modules/test.py b/salt/modules/test.py index fc710255f34b..62d96f52118b 100644 --- a/salt/modules/test.py +++ b/salt/modules/test.py @@ -16,6 +16,7 @@ import salt.utils.functools import salt.utils.hashutils import salt.utils.platform +import salt.utils.versions import salt.version from salt.utils.decorators import depends @@ -675,3 +676,27 @@ def _is_exc(cls): except AttributeError: log.error("No such exception: %s", name) return False + + +def deprecation_warning(): + r""" + Return True, but also produce two DeprecationWarnings. One by date, the + other by the codename - release Oganesson, which should correspond to Salt + 3108. + + CLI Example: + + .. code-block:: bash + + salt \* test.deprecation_warning + """ + # This warn should always stay in Salt. + salt.utils.versions.warn_until( + "Oganesson", + "This is a test deprecation warning by version.", + ) + salt.utils.versions.warn_until_date( + "30000101", + "This is a test deprecation warning by date very far into the future ({date}).", + ) + return True diff --git a/tests/pytests/integration/modules/test_test.py b/tests/pytests/integration/modules/test_test.py new file mode 100644 index 000000000000..82f6432383be --- /dev/null +++ b/tests/pytests/integration/modules/test_test.py @@ -0,0 +1,8 @@ +def test_deprecation_warning_emits_deprecation_warnings(salt_call_cli): + ret = salt_call_cli.run("test.deprecation_warning") + assert ret.stderr.count("DeprecationWarning") >= 2 + assert "This is a test deprecation warning by version." in ret.stderr + assert ( + "This is a test deprecation warning by date very far into the future (3000-01-01)" + in ret.stderr + ) From 0d48439205535e912bdc555a183248820af546df Mon Sep 17 00:00:00 2001 From: Wayne Werner Date: Fri, 6 Jan 2023 13:25:54 -0600 Subject: [PATCH 3/3] Include certain deprecation warnings on tests Due to some of the versions of dependencies we're using, these tests may emit some specific errors. --- tests/pytests/integration/cli/test_batch.py | 28 ++++++++++- .../scenarios/daemons/test_salt_as_daemons.py | 46 ++++++++++++++++++- 2 files changed, 70 insertions(+), 4 deletions(-) diff --git a/tests/pytests/integration/cli/test_batch.py b/tests/pytests/integration/cli/test_batch.py index 230aa6d5cbb2..f5b104ba67b1 100644 --- a/tests/pytests/integration/cli/test_batch.py +++ b/tests/pytests/integration/cli/test_batch.py @@ -179,7 +179,19 @@ def test_batch_retcode(salt_cli, salt_minion, salt_sub_minion, run_timeout): ) assert cmd.returncode == 23 - assert not cmd.stderr + # TODO: Certain platforms will have a warning related to jinja. But + # that's an issue with dependency versions that may be due to the versions + # installed on the test images. When those issues are sorted, this can + # simply `not cmd.stderr`. + assert ( + not cmd.stderr + or cmd.stderr.endswith( + "jinja.py:736: DeprecationWarning: 'contextfunction' is renamed to 'pass_context', the old name will be removed in Jinja 3.1.\n @contextfunction\n" + ) + or cmd.stderr.endswith( + "process.py:54: DeprecationWarning: PY_SSIZE_T_CLEAN will be required for '#' formats\n current = setproctitle.getproctitle()\n" + ) + ) assert "true" in cmd.stdout @@ -198,7 +210,19 @@ def test_multiple_modules_in_batch(salt_cli, salt_minion, salt_sub_minion, run_t ) assert cmd.returncode == 23 - assert not cmd.stderr + # TODO: Certain platforms will have a warning related to setproctitle. But + # that's an issue with dependency versions that may be due to the versions + # installed on the test images. When those issues are sorted, this can + # simply `not cmd.stderr`. + assert ( + not cmd.stderr + or cmd.stderr.endswith( + "process.py:54: DeprecationWarning: PY_SSIZE_T_CLEAN will be required for '#' formats\n current = setproctitle.getproctitle()\n" + ) + or cmd.stderr.endswith( + "jinja.py:736: DeprecationWarning: 'contextfunction' is renamed to 'pass_context', the old name will be removed in Jinja 3.1.\n @contextfunction\n" + ) + ) def test_batch_module_stopping_failed_respond( diff --git a/tests/pytests/scenarios/daemons/test_salt_as_daemons.py b/tests/pytests/scenarios/daemons/test_salt_as_daemons.py index e4dca82ce6ac..4769f261cdac 100644 --- a/tests/pytests/scenarios/daemons/test_salt_as_daemons.py +++ b/tests/pytests/scenarios/daemons/test_salt_as_daemons.py @@ -24,7 +24,28 @@ def test_salt_master_as_daemon(salt_master_factory): pass finally: assert salt_master_factory.impl._terminal_result.stdout == "" - assert salt_master_factory.impl._terminal_result.stderr == "" + # TODO currently setproctitle.getproctitle warns on some platforms. + # It's fine, actually, for that warning to show up, until the + # correct versions are configured for those platforms. When they + # are, the 'if not all(...):' line should be removed and just the + # assertion that `stderr == ""` should be left. + if not any( + ( + all( + text in salt_master_factory.impl._terminal_result.stderr + for text in ("PY_SSIZE_T_CLEAN", "salt/utils/process.py:54") + ), + all( + text in salt_master_factory.impl._terminal_result.stderr + for text in ( + "salt/utils/jinja.py:736", + "contextfunction", + "pass_context", + ) + ), + ) + ): + assert salt_master_factory.impl._terminal_result.stderr == "" assert salt_master_factory.impl._terminal_result.returncode == 0 # We are going to kill the possible child processes based on the entire cmdline @@ -61,7 +82,28 @@ def test_salt_minion_as_daemon(salt_minion_factory): pass finally: assert salt_minion_factory.impl._terminal_result.stdout == "" - assert salt_minion_factory.impl._terminal_result.stderr == "" + # TODO currently setproctitle.getproctitle warns on some platforms. + # It's fine, actually, for that warning to show up, until the + # correct versions are configured for those platforms. When they + # are, the 'if not all(...):' line should be removed and just the + # assertion that `stderr == ""` should be left. + if not any( + ( + all( + text in salt_minion_factory.impl._terminal_result.stderr + for text in ("PY_SSIZE_T_CLEAN", "salt/utils/process.py:54") + ), + all( + text in salt_minion_factory.impl._terminal_result.stderr + for text in ( + "salt/utils/jinja.py:736", + "contextfunction", + "pass_context", + ) + ), + ) + ): + assert salt_minion_factory.impl._terminal_result.stderr == "" assert salt_minion_factory.impl._terminal_result.returncode == 0 # We are going to kill the possible child processes based on the entire cmdline