Skip to content

Commit

Permalink
🐛 Ensure that no-token case is actually logged
Browse files Browse the repository at this point in the history
Prior to this patch, there was an unreachable code path that seems
to have been intended for marking the case when no token is provided
with the `"NOTOKEN"` string in the debug log output. However the logic
behind choosing to show it contained two opposite checks. Said place
in the logging utils, didn't have any coverage either.

This change updates the code to make all of its branches reachable.
Moreover, it adds test cases for all the branches that remain in the
implementation.
  • Loading branch information
webknjaz committed Jun 26, 2024
1 parent 992826d commit 1772372
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 5 deletions.
11 changes: 6 additions & 5 deletions codecov_cli/helpers/logging_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,12 @@ def format(self, record):
f"{prefix} - {asctime} -- {x}" for x in msg.splitlines()
)
if hasattr(record, "extra_log_attributes"):
token = record.extra_log_attributes.get("token")
if token:
record.extra_log_attributes["token"] = (
"NOTOKEN" if not token else (str(token)[:1] + 18 * "*")
)
passed_token_attribute = record.extra_log_attributes.get("token")
record.extra_log_attributes["token"] = (
"NOTOKEN"
if passed_token_attribute is None
else (str(passed_token_attribute)[:1] + 18 * "*")
)
msg += " --- " + json.dumps(
record.extra_log_attributes, cls=JsonEncoder
)
Expand Down
83 changes: 83 additions & 0 deletions tests/commands/test_upload_token_discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,86 @@ def test_no_cli_token_config_fallback(
CliRunner().invoke(cli, ["do-upload", "--commit-sha=deadbeef"], obj={})

assert do_upload_cmd_spy.call_args[-1]["token"] == "sentinel-value"


def test_no_token_anywhere(
mocker: MockerFixture,
monkeypatch: MonkeyPatch,
tmp_path: Path,
) -> None:
"""Test that a missing token produces `NOTOKEN` in logs."""
# NOTE: The pytest's `caplog` fixture is not used in this test as it
# NOTE: doesn't play well with Click's testing CLI runner, and does
# NOTE: not capture any log entries for mysterious reasons.
#
# Refs:
# * https://github.com/pallets/click/issues/2573#issuecomment-1649773563
# * https://github.com/pallets/click/issues/1763#issuecomment-767687608
monkeypatch.chdir(tmp_path)

mocker.patch.object(upload, "do_upload_logic")
do_upload_cmd_spy = mocker.spy(upload, "do_upload_logic")
debug_log_spy = mocker.spy(upload.logger, "debug")

cov_upload_cmd_output = (
CliRunner()
.invoke(
cli,
[
"-v", # <- NOTE: this is the only way to turn on debug logger
"do-upload",
"--commit-sha=deadbeef",
],
obj={},
)
.output
)

assert (
debug_log_spy.call_args[1]["extra"]["extra_log_attributes"]["token"]
== "NOTOKEN"
)
assert '"token": "NOTOKEN"' in cov_upload_cmd_output
assert do_upload_cmd_spy.call_args[-1]["token"] is None


def test_cli_token_masked_in_logs(
mocker: MockerFixture,
monkeypatch: MonkeyPatch,
tmp_path: Path,
) -> None:
"""Test that a present token is masked in logs."""
# NOTE: The pytest's `caplog` fixture is not used in this test as it
# NOTE: doesn't play well with Click's testing CLI runner, and does
# NOTE: not capture any log entries for mysterious reasons.
#
# Refs:
# * https://github.com/pallets/click/issues/2573#issuecomment-1649773563
# * https://github.com/pallets/click/issues/1763#issuecomment-767687608
monkeypatch.chdir(tmp_path)

mocker.patch.object(upload, "do_upload_logic")
do_upload_cmd_spy = mocker.spy(upload, "do_upload_logic")
debug_log_spy = mocker.spy(upload.logger, "debug")

cov_upload_cmd_output = (
CliRunner()
.invoke(
cli,
[
"-v", # <- NOTE: this is the only way to turn on debug logger
"do-upload",
"--commit-sha=deadbeef",
"--token=sentinel-value",
],
obj={},
)
.output
)

assert (
debug_log_spy.call_args[1]["extra"]["extra_log_attributes"]["token"]
== "s" + "*" * 18
)
assert f'"token": "s{"*" * 18}"' in cov_upload_cmd_output
assert do_upload_cmd_spy.call_args[-1]["token"] == "sentinel-value"

0 comments on commit 1772372

Please sign in to comment.