borgmatic-collective/borgmatic
borgmatic-collective/borgmatic
12
63
Fork 16
Code Issues 49 Pull Requests 1 Actions Packages Releases 133 Activity

test_should_do_markup_respects_config_value fails when NO_COLOR env variable is set #943

New Issue
Closed
opened 2024-11-19 09:56:06 +00:00 by arkamar · 1 comment
arkamar commented 2024-11-19 09:56:06 +00:00
Copy Link

Gentoo CI system detected following test failure:

FAILED tests/unit/test_logger.py::test_should_do_markup_respects_config_value - AssertionError: assert False is True
 +  where False = <function should_do_markup at 0x7fbc12d03240>(no_color=False, configs={'foo.yaml': {'color': True}})
 +    where <function should_do_markup at 0x7fbc12d03240> = module.should_do_markup
========== 1 failed, 1654 passed, 1 deselected, 28 warnings in 7.95s ===========

The issue can be reproduced when the tests are run with NO_COLOR=1 environmental variable set, see also https://bugs.gentoo.org/943393. This happens because should_do_markup returns False when NO_COLOR is set

borgmatic/logger.py Lines 44 to 45 in d09b4c72a9
if os.environ.get('NO_COLOR', None):
return False

I think it would be appropriate to sanitize env variables before the test.

Gentoo CI system detected following test failure: ``` FAILED tests/unit/test_logger.py::test_should_do_markup_respects_config_value - AssertionError: assert False is True + where False = <function should_do_markup at 0x7fbc12d03240>(no_color=False, configs={'foo.yaml': {'color': True}}) + where <function should_do_markup at 0x7fbc12d03240> = module.should_do_markup ========== 1 failed, 1654 passed, 1 deselected, 28 warnings in 7.95s =========== ``` The issue can be reproduced when the tests are run with `NO_COLOR=1` environmental variable set, see also https://bugs.gentoo.org/943393. This happens because `should_do_markup` returns `False` when `NO_COLOR` is set https://projects.torsion.org/borgmatic-collective/borgmatic/src/commit/d09b4c72a9bd77802297e42b83760e6a0ce84625/borgmatic/logger.py#L44-L45 I think it would be appropriate to sanitize env variables before the test.
witten commented 2024-11-19 16:49:20 +00:00
Owner
Copy Link

Thanks for bringing this to my attention. IMO, unit tests shouldn't even be looking at environment variables to begin with. Anyway, this has been fixed in main and will be part of the next release!

Thanks for bringing this to my attention. IMO, unit tests shouldn't even be looking at environment variables to begin with. Anyway, this has been fixed in main and will be part of the next release!
👍 1
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
blocked
bug
data loss
design finalized
good first issue
new feature area
question / support
security
waiting for response
No Label
Milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: borgmatic-collective/borgmatic#943
No description provided.
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?