From 41ac11ab3e0ff474693e470bc0e2fd7f92805cf4 Mon Sep 17 00:00:00 2001 From: Juliya Smith Date: Thu, 19 Sep 2024 10:58:48 -0500 Subject: [PATCH 1/6] fix: ape test -vvv --- src/ape/logging.py | 23 ++++++++++++++++++----- tests/integration/cli/test_test.py | 10 ++++++++++ 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/ape/logging.py b/src/ape/logging.py index fd4161c90d..a41049f04e 100644 --- a/src/ape/logging.py +++ b/src/ape/logging.py @@ -127,6 +127,7 @@ def __init__( self.info = _logger.info self.debug = _logger.debug self._logger = _logger + self._did_parse_sys_argv = False self._load_from_sys_argv() self.fmt = fmt @@ -145,22 +146,34 @@ def _load_from_sys_argv(self, default: Optional[Union[str, int, LogLevel]] = Non """ Load from sys.argv to beat race condition with `click`. """ + if self._did_parse_sys_argv: + # Already parsed. + return log_level = _get_level(level=default) level_names = [lvl.name for lvl in LogLevel] - for arg_i in range(len(sys.argv) - 1): + + # Minus 2 because if `-v` is the last arg, it is not our verbosity `-v`. + num_args = len(sys.argv) - 2 + + for arg_i in range(num_args): if sys.argv[arg_i] == "-v" or sys.argv[arg_i] == "--verbosity": - level = _get_level(sys.argv[arg_i + 1].upper()) + try: + level = _get_level(sys.argv[arg_i + 1].upper()) + except Exception: + # Let it fail in a better spot, or is not our level. + continue if level in level_names: + self._sys_argv = level log_level = level break else: - names_str = f"{', '.join(level_names[:-1])}, or {level_names[-1]}" - self._logger.error(f"Must be one of '{names_str}', not '{level}'.") - sys.exit(2) + # Not our level. + continue self.set_level(log_level) + self._did_parse_sys_argv = True @property def level(self) -> int: diff --git a/tests/integration/cli/test_test.py b/tests/integration/cli/test_test.py index eadaefc443..1c2d092aba 100644 --- a/tests/integration/cli/test_test.py +++ b/tests/integration/cli/test_test.py @@ -232,6 +232,16 @@ def test_verbosity(runner, ape_cli): assert result.exit_code == 0, result.output +@skip_projects_except("test") +def test_vvv(runner, ape_cli): + """ + Showing you can somehow use pytest's -v flag without + messing up Ape. + """ + result = runner.invoke(ape_cli, ("test", "-vvv", "--fixtures")) + assert result.exit_code == 0, result.output + + @skip_projects_except("test", "with-contracts") def test_fixture_docs(setup_pytester, integ_project, pytester, eth_tester_provider): _ = eth_tester_provider # Ensure using EthTester for this test. From 0473208187e40d2a751c66abec962b370b1bc379 Mon Sep 17 00:00:00 2001 From: Juliya Smith Date: Thu, 19 Sep 2024 11:12:59 -0500 Subject: [PATCH 2/6] test: fix other tests --- tests/functional/test_logging.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/functional/test_logging.py b/tests/functional/test_logging.py index 9bf53cd768..7747a087a1 100644 --- a/tests/functional/test_logging.py +++ b/tests/functional/test_logging.py @@ -33,6 +33,7 @@ def test_info_level_higher(simple_runner): def cmd(cli_ctx): cli_ctx.logger.info("this is a test") + logger._did_parse_sys_argv = False result = simple_runner.invoke(group_for_testing, ("cmd", "-v", "WARNING")) # You don't get INFO log when log level is higher @@ -57,6 +58,7 @@ def test_warning_level_higher(simple_runner): def cmd(cli_ctx): cli_ctx.logger.warning("this is a test") + logger._did_parse_sys_argv = False result = simple_runner.invoke(group_for_testing, ("cmd", "-v", "ERROR")) assert "WARNING" not in result.output assert "this is a test" not in result.output @@ -71,6 +73,7 @@ def test_success(simple_runner): def cmd(cli_ctx): cli_ctx.logger.success("this is a test") + logger._did_parse_sys_argv = False result = simple_runner.invoke(group_for_testing, "cmd") assert "SUCCESS" in result.output assert "this is a test" in result.output @@ -82,6 +85,7 @@ def test_success_level_higher(simple_runner): def cmd(cli_ctx): cli_ctx.logger.success("this is a test") + logger._did_parse_sys_argv = False result = simple_runner.invoke(group_for_testing, ("cmd", "-v", "WARNING")) assert "SUCCESS" not in result.output assert "this is a test" not in result.output From 909fde0c338f208d952ac1ebe096e5f5d36afa59 Mon Sep 17 00:00:00 2001 From: Juliya Smith Date: Thu, 19 Sep 2024 11:14:46 -0500 Subject: [PATCH 3/6] fix: better fix for already parsed --- src/ape/cli/options.py | 5 ++++- tests/functional/test_logging.py | 4 ---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/ape/cli/options.py b/src/ape/cli/options.py index 0daacd9c3a..e484db28c3 100644 --- a/src/ape/cli/options.py +++ b/src/ape/cli/options.py @@ -77,7 +77,10 @@ def set_level(ctx, param, value): if value.startswith("LOGLEVEL."): value = value.split(".")[-1].strip() - cli_logger._load_from_sys_argv(default=value) + if cli_logger._did_parse_sys_argv: + cli_logger.set_level(value) + else: + cli_logger._load_from_sys_argv(default=value) level_names = [lvl.name for lvl in LogLevel] names_str = f"{', '.join(level_names[:-1])}, or {level_names[-1]}" diff --git a/tests/functional/test_logging.py b/tests/functional/test_logging.py index 7747a087a1..9bf53cd768 100644 --- a/tests/functional/test_logging.py +++ b/tests/functional/test_logging.py @@ -33,7 +33,6 @@ def test_info_level_higher(simple_runner): def cmd(cli_ctx): cli_ctx.logger.info("this is a test") - logger._did_parse_sys_argv = False result = simple_runner.invoke(group_for_testing, ("cmd", "-v", "WARNING")) # You don't get INFO log when log level is higher @@ -58,7 +57,6 @@ def test_warning_level_higher(simple_runner): def cmd(cli_ctx): cli_ctx.logger.warning("this is a test") - logger._did_parse_sys_argv = False result = simple_runner.invoke(group_for_testing, ("cmd", "-v", "ERROR")) assert "WARNING" not in result.output assert "this is a test" not in result.output @@ -73,7 +71,6 @@ def test_success(simple_runner): def cmd(cli_ctx): cli_ctx.logger.success("this is a test") - logger._did_parse_sys_argv = False result = simple_runner.invoke(group_for_testing, "cmd") assert "SUCCESS" in result.output assert "this is a test" in result.output @@ -85,7 +82,6 @@ def test_success_level_higher(simple_runner): def cmd(cli_ctx): cli_ctx.logger.success("this is a test") - logger._did_parse_sys_argv = False result = simple_runner.invoke(group_for_testing, ("cmd", "-v", "WARNING")) assert "SUCCESS" not in result.output assert "this is a test" not in result.output From dd9700830d95fb06c6615f761eb82061fd75d431 Mon Sep 17 00:00:00 2001 From: Juliya Smith Date: Thu, 19 Sep 2024 11:15:41 -0500 Subject: [PATCH 4/6] docs: add comment --- src/ape/cli/options.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ape/cli/options.py b/src/ape/cli/options.py index e484db28c3..3890187597 100644 --- a/src/ape/cli/options.py +++ b/src/ape/cli/options.py @@ -80,6 +80,7 @@ def set_level(ctx, param, value): if cli_logger._did_parse_sys_argv: cli_logger.set_level(value) else: + # Avoid callback without fully parsing sys argv first. cli_logger._load_from_sys_argv(default=value) level_names = [lvl.name for lvl in LogLevel] From 834d957045b1c4598cffa28cdd00dca7c6c491e4 Mon Sep 17 00:00:00 2001 From: Juliya Smith Date: Thu, 19 Sep 2024 13:47:25 -0500 Subject: [PATCH 5/6] fix: simpler works better --- src/ape/cli/options.py | 6 +----- tests/integration/cli/test_test.py | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/ape/cli/options.py b/src/ape/cli/options.py index 3890187597..0daacd9c3a 100644 --- a/src/ape/cli/options.py +++ b/src/ape/cli/options.py @@ -77,11 +77,7 @@ def set_level(ctx, param, value): if value.startswith("LOGLEVEL."): value = value.split(".")[-1].strip() - if cli_logger._did_parse_sys_argv: - cli_logger.set_level(value) - else: - # Avoid callback without fully parsing sys argv first. - cli_logger._load_from_sys_argv(default=value) + cli_logger._load_from_sys_argv(default=value) level_names = [lvl.name for lvl in LogLevel] names_str = f"{', '.join(level_names[:-1])}, or {level_names[-1]}" diff --git a/tests/integration/cli/test_test.py b/tests/integration/cli/test_test.py index 1c2d092aba..a96e999fb6 100644 --- a/tests/integration/cli/test_test.py +++ b/tests/integration/cli/test_test.py @@ -238,7 +238,7 @@ def test_vvv(runner, ape_cli): Showing you can somehow use pytest's -v flag without messing up Ape. """ - result = runner.invoke(ape_cli, ("test", "-vvv", "--fixtures")) + result = runner.invoke(ape_cli, ("test", "-vvv", "--fixtures"), catch_exceptions=False) assert result.exit_code == 0, result.output From 0148f36a1f927f775396a6dfcbfaf9eb065bd264 Mon Sep 17 00:00:00 2001 From: Juliya Smith Date: Thu, 19 Sep 2024 16:31:23 -0500 Subject: [PATCH 6/6] test: fix tests --- tests/functional/test_logging.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/functional/test_logging.py b/tests/functional/test_logging.py index 9bf53cd768..bbd3b2d71b 100644 --- a/tests/functional/test_logging.py +++ b/tests/functional/test_logging.py @@ -33,6 +33,7 @@ def test_info_level_higher(simple_runner): def cmd(cli_ctx): cli_ctx.logger.info("this is a test") + logger._did_parse_sys_argv = False result = simple_runner.invoke(group_for_testing, ("cmd", "-v", "WARNING")) # You don't get INFO log when log level is higher @@ -57,6 +58,7 @@ def test_warning_level_higher(simple_runner): def cmd(cli_ctx): cli_ctx.logger.warning("this is a test") + logger._did_parse_sys_argv = False result = simple_runner.invoke(group_for_testing, ("cmd", "-v", "ERROR")) assert "WARNING" not in result.output assert "this is a test" not in result.output @@ -71,6 +73,7 @@ def test_success(simple_runner): def cmd(cli_ctx): cli_ctx.logger.success("this is a test") + logger._did_parse_sys_argv = False result = simple_runner.invoke(group_for_testing, "cmd") assert "SUCCESS" in result.output assert "this is a test" in result.output @@ -82,6 +85,7 @@ def test_success_level_higher(simple_runner): def cmd(cli_ctx): cli_ctx.logger.success("this is a test") + logger._did_parse_sys_argv = False result = simple_runner.invoke(group_for_testing, ("cmd", "-v", "WARNING")) assert "SUCCESS" not in result.output assert "this is a test" not in result.output @@ -97,6 +101,7 @@ def cmd(cli_ctx): finally: cli_ctx.logger.format() + logger._did_parse_sys_argv = False result = simple_runner.invoke(group_for_testing, ("cmd", "-v", "SUCCESS")) assert "SUCCESS" not in result.output