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 25, 2024
1 parent 65a32bc commit a06cbdf
Show file tree
Hide file tree
Showing 2 changed files with 99 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
93 changes: 93 additions & 0 deletions tests/commands/test_upload_token_discovery.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
"""Tests ensuring that an env-provided token can be found."""

from pathlib import Path

from click.testing import CliRunner
from pytest import MonkeyPatch
from pytest_mock import MockerFixture

from codecov_cli.commands import upload
from codecov_cli.main import cli


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 a06cbdf

Please sign in to comment.