From 3cfffb2066102f95e16049d0c74a7b3c41121df9 Mon Sep 17 00:00:00 2001 From: Masayuki Igawa Date: Wed, 6 Apr 2022 09:29:36 +0900 Subject: [PATCH 1/3] Deprecate repo_type parameter in get_repo_open and get_repo_initialise This commit deprecates `repo_type` parameter in `get_repo_open` and `get_repo_initialise` functions. As we discussed in PR#318 [1], these should be deprecated before 4.0.0 release. [1] https://github.com/mtreinish/stestr/pull/318 --- stestr/commands/failing.py | 2 +- stestr/commands/history.py | 6 +++--- stestr/commands/init.py | 2 +- stestr/commands/last.py | 2 +- stestr/commands/load.py | 4 ++-- stestr/commands/run.py | 6 +++--- stestr/commands/slowest.py | 2 +- stestr/config_file.py | 2 +- stestr/repository/util.py | 26 ++++++++++++++++++++++---- stestr/tests/test_config_file.py | 3 +-- 10 files changed, 36 insertions(+), 19 deletions(-) diff --git a/stestr/commands/failing.py b/stestr/commands/failing.py index 809d707..6302a20 100644 --- a/stestr/commands/failing.py +++ b/stestr/commands/failing.py @@ -103,7 +103,7 @@ def failing(repo_url=None, list_tests=False, subunit=False, for failures. :rtype: int """ - repo = util.get_repo_open('file', repo_url) + repo = util.get_repo_open(repo_url) run = repo.get_failing() if subunit: return _show_subunit(run) diff --git a/stestr/commands/history.py b/stestr/commands/history.py index 4a1d5eb..0eb651a 100644 --- a/stestr/commands/history.py +++ b/stestr/commands/history.py @@ -228,7 +228,7 @@ def history_list(repo_url=None, show_metadata=False, else: field_names = ('Run ID', 'Passed', 'Runtime', 'Date') try: - repo = util.get_repo_open('file', repo_url) + repo = util.get_repo_open(repo_url) except abstract.RepositoryNotFound as e: stdout.write(str(e) + '\n') return 1 @@ -285,7 +285,7 @@ def history_show(run_id, repo_url=None, subunit_out=False, :rtype: int """ try: - repo = util.get_repo_open('file', repo_url) + repo = util.get_repo_open(repo_url) except abstract.RepositoryNotFound as e: stdout.write(str(e) + '\n') return 1 @@ -352,7 +352,7 @@ def history_remove(run_id, repo_url=None, stdout=sys.stdout): :rtype: int """ try: - repo = util.get_repo_open('file', repo_url) + repo = util.get_repo_open(repo_url) except abstract.RepositoryNotFound as e: stdout.write(str(e) + '\n') return 1 diff --git a/stestr/commands/init.py b/stestr/commands/init.py index 0540c31..467e131 100644 --- a/stestr/commands/init.py +++ b/stestr/commands/init.py @@ -43,7 +43,7 @@ def init(repo_url=None, stdout=sys.stdout): :rtype: int """ try: - util.get_repo_initialise('file', repo_url) + util.get_repo_initialise(repo_url) except OSError as e: if e.errno != errno.EEXIST: raise diff --git a/stestr/commands/last.py b/stestr/commands/last.py index 230f593..cdcb781 100644 --- a/stestr/commands/last.py +++ b/stestr/commands/last.py @@ -146,7 +146,7 @@ def last(repo_url=None, subunit_out=False, pretty_out=True, :rtype: int """ try: - repo = util.get_repo_open('file', repo_url) + repo = util.get_repo_open(repo_url) except abstract.RepositoryNotFound as e: stdout.write(str(e) + '\n') return 1 diff --git a/stestr/commands/load.py b/stestr/commands/load.py index 4268822..7e54013 100644 --- a/stestr/commands/load.py +++ b/stestr/commands/load.py @@ -173,11 +173,11 @@ def load(force_init=False, in_streams=None, :rtype: int """ try: - repo = util.get_repo_open('file', repo_url) + repo = util.get_repo_open(repo_url) except repository.RepositoryNotFound: if force_init: try: - repo = util.get_repo_initialise('file', repo_url) + repo = util.get_repo_initialise(repo_url) except OSError as e: if e.errno != errno.EEXIST: raise diff --git a/stestr/commands/run.py b/stestr/commands/run.py index f073856..07b8a7f 100644 --- a/stestr/commands/run.py +++ b/stestr/commands/run.py @@ -357,7 +357,7 @@ def run_command(config='.stestr.conf', :rtype: int """ try: - repo = util.get_repo_open('file', repo_url) + repo = util.get_repo_open(repo_url) # If a repo is not found, and there a stestr config exists just create it except repository.RepositoryNotFound: if not os.path.isfile(config) and not test_path: @@ -380,7 +380,7 @@ def run_command(config='.stestr.conf', stdout.write(msg) exit(1) try: - repo = util.get_repo_initialise('file', repo_url) + repo = util.get_repo_initialise(repo_url) except OSError as e: if e.errno != errno.EEXIST: raise @@ -634,7 +634,7 @@ def run_tests(): # the result from the repository because load() returns 0 # always on subunit output if subunit_out: - repo = util.get_repo_open('file', repo_url) + repo = util.get_repo_open(repo_url) summary = testtools.StreamSummary() last_run = repo.get_latest_run().get_subunit_stream() stream = subunit.ByteStreamToStreamResult(last_run) diff --git a/stestr/commands/slowest.py b/stestr/commands/slowest.py index 6779165..fe1d4a9 100644 --- a/stestr/commands/slowest.py +++ b/stestr/commands/slowest.py @@ -79,7 +79,7 @@ def slowest(repo_url=None, show_all=False, :rtype: int """ - repo = util.get_repo_open('file', repo_url) + repo = util.get_repo_open(repo_url) try: latest_id = repo.latest_id() except KeyError: diff --git a/stestr/config_file.py b/stestr/config_file.py index 52e964d..916a9fa 100644 --- a/stestr/config_file.py +++ b/stestr/config_file.py @@ -164,7 +164,7 @@ def group_callback(test_id, regex=re.compile(group_regex)): else: group_callback = None # Handle the results repository - repository = util.get_repo_open('file', repo_url) + repository = util.get_repo_open(repo_url) return test_processor.TestProcessorFixture( test_ids, command, listopt, idoption, repository, test_filters=regexes, group_callback=group_callback, serial=serial, diff --git a/stestr/repository/util.py b/stestr/repository/util.py index a08dfa2..4341cc6 100644 --- a/stestr/repository/util.py +++ b/stestr/repository/util.py @@ -12,6 +12,8 @@ import importlib import os +import sys +import warnings def _get_default_repo_url(repo_type): @@ -22,26 +24,42 @@ def _get_default_repo_url(repo_type): return repo_url -def get_repo_open(repo_type, repo_url=None): +def get_repo_open(repo_type=None, repo_url=None): """Return an already initialized repo object given the parameters - :param str repo_type: The repo module to use for the returned repo + :param str repo_type: DEPRECATED - The repo module to use for the returned + repo :param str repo_url: An optional repo url, if one is not specified the default $CWD/.stestr will be used. """ + if repo_type is not None: + msg = ("WARNING: Specifying repository type is deprecated and will be " + "removed in future release.\n") + sys.stderr.write(msg) + warnings.warn(msg, DeprecationWarning, stacklevel=3) + else: + repo_type = 'file' repo_module = importlib.import_module('stestr.repository.' + repo_type) if not repo_url: repo_url = _get_default_repo_url(repo_type) return repo_module.RepositoryFactory().open(repo_url) -def get_repo_initialise(repo_type, repo_url=None): +def get_repo_initialise(repo_type=None, repo_url=None): """Return a newly initialized repo object given the parameters - :param str repo_type: The repo module to use for the returned repo + :param str repo_type: DEPRECATED - The repo module to use for the returned + repo :param str repo_url: An optional repo url, if one is not specified the default $CWD/.stestr will be used. """ + if repo_type is not None: + msg = ("WARNING: Specifying repository type is deprecated and will be " + "removed in future release.\n") + sys.stderr.write(msg) + warnings.warn(msg, DeprecationWarning, stacklevel=3) + else: + repo_type = 'file' repo_module = importlib.import_module('stestr.repository.' + repo_type) if not repo_url: repo_url = _get_default_repo_url(repo_type) diff --git a/stestr/tests/test_config_file.py b/stestr/tests/test_config_file.py index 219ac04..7acdb69 100644 --- a/stestr/tests/test_config_file.py +++ b/stestr/tests/test_config_file.py @@ -51,8 +51,7 @@ def _check_get_run_command(self, mock_sys, mock_TestProcessorFixture, parallel_class=parallel_class) self.assertEqual(mock_TestProcessorFixture.return_value, fixture) - mock_get_repo_open.assert_called_once_with('file', - None) + mock_get_repo_open.assert_called_once_with(None) command = '"%s" -m stestr.subunit_runner.run discover -t "%s" "%s" ' \ '$LISTOPT $IDOPTION' % (expected_python, 'fake_top_dir', 'fake_test_path') From cad322985a0b29d4434cbd47ceec38fa367a2a7a Mon Sep 17 00:00:00 2001 From: Masayuki Igawa Date: Wed, 13 Apr 2022 08:22:37 +0900 Subject: [PATCH 2/3] Remove warning message for stderr This commit removes warning messages for the stderr because these interfaces are only for python library interface not for CLI. We don't need to show them to users. --- stestr/repository/util.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/stestr/repository/util.py b/stestr/repository/util.py index 4341cc6..1967168 100644 --- a/stestr/repository/util.py +++ b/stestr/repository/util.py @@ -33,9 +33,6 @@ def get_repo_open(repo_type=None, repo_url=None): default $CWD/.stestr will be used. """ if repo_type is not None: - msg = ("WARNING: Specifying repository type is deprecated and will be " - "removed in future release.\n") - sys.stderr.write(msg) warnings.warn(msg, DeprecationWarning, stacklevel=3) else: repo_type = 'file' @@ -54,9 +51,6 @@ def get_repo_initialise(repo_type=None, repo_url=None): default $CWD/.stestr will be used. """ if repo_type is not None: - msg = ("WARNING: Specifying repository type is deprecated and will be " - "removed in future release.\n") - sys.stderr.write(msg) warnings.warn(msg, DeprecationWarning, stacklevel=3) else: repo_type = 'file' From 011c84d2e52b4c9dd5da784f691b6e4273c64c12 Mon Sep 17 00:00:00 2001 From: Masayuki Igawa Date: Wed, 13 Apr 2022 08:37:57 +0900 Subject: [PATCH 3/3] Fix typos on all the updated calls This commit fixes to use the updated call correctly. --- stestr/commands/failing.py | 2 +- stestr/commands/history.py | 6 +++--- stestr/commands/init.py | 2 +- stestr/commands/last.py | 2 +- stestr/commands/load.py | 4 ++-- stestr/commands/run.py | 6 +++--- stestr/commands/slowest.py | 2 +- stestr/config_file.py | 2 +- stestr/repository/util.py | 5 ++++- stestr/tests/test_config_file.py | 2 +- 10 files changed, 18 insertions(+), 15 deletions(-) diff --git a/stestr/commands/failing.py b/stestr/commands/failing.py index 6302a20..d0a42b9 100644 --- a/stestr/commands/failing.py +++ b/stestr/commands/failing.py @@ -103,7 +103,7 @@ def failing(repo_url=None, list_tests=False, subunit=False, for failures. :rtype: int """ - repo = util.get_repo_open(repo_url) + repo = util.get_repo_open(repo_url=repo_url) run = repo.get_failing() if subunit: return _show_subunit(run) diff --git a/stestr/commands/history.py b/stestr/commands/history.py index 0eb651a..2a17fbd 100644 --- a/stestr/commands/history.py +++ b/stestr/commands/history.py @@ -228,7 +228,7 @@ def history_list(repo_url=None, show_metadata=False, else: field_names = ('Run ID', 'Passed', 'Runtime', 'Date') try: - repo = util.get_repo_open(repo_url) + repo = util.get_repo_open(repo_url=repo_url) except abstract.RepositoryNotFound as e: stdout.write(str(e) + '\n') return 1 @@ -285,7 +285,7 @@ def history_show(run_id, repo_url=None, subunit_out=False, :rtype: int """ try: - repo = util.get_repo_open(repo_url) + repo = util.get_repo_open(repo_url=repo_url) except abstract.RepositoryNotFound as e: stdout.write(str(e) + '\n') return 1 @@ -352,7 +352,7 @@ def history_remove(run_id, repo_url=None, stdout=sys.stdout): :rtype: int """ try: - repo = util.get_repo_open(repo_url) + repo = util.get_repo_open(repo_url=repo_url) except abstract.RepositoryNotFound as e: stdout.write(str(e) + '\n') return 1 diff --git a/stestr/commands/init.py b/stestr/commands/init.py index 467e131..0310589 100644 --- a/stestr/commands/init.py +++ b/stestr/commands/init.py @@ -43,7 +43,7 @@ def init(repo_url=None, stdout=sys.stdout): :rtype: int """ try: - util.get_repo_initialise(repo_url) + util.get_repo_initialise(repo_url=repo_url) except OSError as e: if e.errno != errno.EEXIST: raise diff --git a/stestr/commands/last.py b/stestr/commands/last.py index cdcb781..1b45cb9 100644 --- a/stestr/commands/last.py +++ b/stestr/commands/last.py @@ -146,7 +146,7 @@ def last(repo_url=None, subunit_out=False, pretty_out=True, :rtype: int """ try: - repo = util.get_repo_open(repo_url) + repo = util.get_repo_open(repo_url=repo_url) except abstract.RepositoryNotFound as e: stdout.write(str(e) + '\n') return 1 diff --git a/stestr/commands/load.py b/stestr/commands/load.py index 7e54013..3e26b99 100644 --- a/stestr/commands/load.py +++ b/stestr/commands/load.py @@ -173,11 +173,11 @@ def load(force_init=False, in_streams=None, :rtype: int """ try: - repo = util.get_repo_open(repo_url) + repo = util.get_repo_open(repo_url=repo_url) except repository.RepositoryNotFound: if force_init: try: - repo = util.get_repo_initialise(repo_url) + repo = util.get_repo_initialise(repo_url=repo_url) except OSError as e: if e.errno != errno.EEXIST: raise diff --git a/stestr/commands/run.py b/stestr/commands/run.py index 07b8a7f..9d5e2ac 100644 --- a/stestr/commands/run.py +++ b/stestr/commands/run.py @@ -357,7 +357,7 @@ def run_command(config='.stestr.conf', :rtype: int """ try: - repo = util.get_repo_open(repo_url) + repo = util.get_repo_open(repo_url=repo_url) # If a repo is not found, and there a stestr config exists just create it except repository.RepositoryNotFound: if not os.path.isfile(config) and not test_path: @@ -380,7 +380,7 @@ def run_command(config='.stestr.conf', stdout.write(msg) exit(1) try: - repo = util.get_repo_initialise(repo_url) + repo = util.get_repo_initialise(repo_url=repo_url) except OSError as e: if e.errno != errno.EEXIST: raise @@ -634,7 +634,7 @@ def run_tests(): # the result from the repository because load() returns 0 # always on subunit output if subunit_out: - repo = util.get_repo_open(repo_url) + repo = util.get_repo_open(repo_url=repo_url) summary = testtools.StreamSummary() last_run = repo.get_latest_run().get_subunit_stream() stream = subunit.ByteStreamToStreamResult(last_run) diff --git a/stestr/commands/slowest.py b/stestr/commands/slowest.py index fe1d4a9..e93cebc 100644 --- a/stestr/commands/slowest.py +++ b/stestr/commands/slowest.py @@ -79,7 +79,7 @@ def slowest(repo_url=None, show_all=False, :rtype: int """ - repo = util.get_repo_open(repo_url) + repo = util.get_repo_open(repo_url=repo_url) try: latest_id = repo.latest_id() except KeyError: diff --git a/stestr/config_file.py b/stestr/config_file.py index 916a9fa..10730ad 100644 --- a/stestr/config_file.py +++ b/stestr/config_file.py @@ -164,7 +164,7 @@ def group_callback(test_id, regex=re.compile(group_regex)): else: group_callback = None # Handle the results repository - repository = util.get_repo_open(repo_url) + repository = util.get_repo_open(repo_url=repo_url) return test_processor.TestProcessorFixture( test_ids, command, listopt, idoption, repository, test_filters=regexes, group_callback=group_callback, serial=serial, diff --git a/stestr/repository/util.py b/stestr/repository/util.py index 1967168..aba0bb3 100644 --- a/stestr/repository/util.py +++ b/stestr/repository/util.py @@ -12,7 +12,6 @@ import importlib import os -import sys import warnings @@ -33,6 +32,8 @@ def get_repo_open(repo_type=None, repo_url=None): default $CWD/.stestr will be used. """ if repo_type is not None: + msg = ("WARNING: Specifying repository type is deprecated and will be " + "removed in future release.\n") warnings.warn(msg, DeprecationWarning, stacklevel=3) else: repo_type = 'file' @@ -51,6 +52,8 @@ def get_repo_initialise(repo_type=None, repo_url=None): default $CWD/.stestr will be used. """ if repo_type is not None: + msg = ("WARNING: Specifying repository type is deprecated and will be " + "removed in future release.\n") warnings.warn(msg, DeprecationWarning, stacklevel=3) else: repo_type = 'file' diff --git a/stestr/tests/test_config_file.py b/stestr/tests/test_config_file.py index 7acdb69..c790937 100644 --- a/stestr/tests/test_config_file.py +++ b/stestr/tests/test_config_file.py @@ -51,7 +51,7 @@ def _check_get_run_command(self, mock_sys, mock_TestProcessorFixture, parallel_class=parallel_class) self.assertEqual(mock_TestProcessorFixture.return_value, fixture) - mock_get_repo_open.assert_called_once_with(None) + mock_get_repo_open.assert_called_once_with(repo_url=None) command = '"%s" -m stestr.subunit_runner.run discover -t "%s" "%s" ' \ '$LISTOPT $IDOPTION' % (expected_python, 'fake_top_dir', 'fake_test_path')