Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CI #51

Merged
merged 2 commits into from
Jun 11, 2021
Merged

Fix CI #51

merged 2 commits into from
Jun 11, 2021

Conversation

hugovk
Copy link
Contributor

@hugovk hugovk commented Dec 30, 2020

Some time between 1 December and 10 December test_prompt_for_confirmation started failing on the CI:

        # Test that the default reply is shown in uppercase.
        with PatchedAttribute(prompts, 'interactive_prompt', lambda p: 'y'):
            for default_value, expected_text in (True, 'Y/n'), (False, 'y/N'), (None, 'y/n'):
                with CaptureOutput(merged=True) as capturer:
                    assert prompt_for_confirmation("Are you sure?", default=default_value) is True
>                   assert expected_text in capturer.get_text()
E                   AssertionError: assert 'Y/n' in '\n'
E                    +  where '\n' = <bound method CaptureOutput.get_text of <humanfriendly.testing.CaptureOutput object at 0x7f4e25f69a90>>()
E                    +    where <bound method CaptureOutput.get_text of <humanfriendly.testing.CaptureOutput object at 0x7f4e25f69a90>> = <humanfriendly.testing.CaptureOutput object at 0x7f4e25f69a90>.get_text

The same failure happens if I try the older commit which worked before: 44cbc41.

Checking the changed dependencies between those two and through a process of trial and error, it passes with coloredlogs==14.0 and fails with coloredlogs==14.1.

Here's a diff of those releases: xolox/python-coloredlogs@14.0...14.1

In the meantime, let's pin.

@hugovk hugovk changed the title Temporarily pin coloredlogs <= 14.0 to fix test_prompt_for_confirmation Fix CI: Temporarily pin coloredlogs <= 14.0 Dec 30, 2020
@hugovk hugovk changed the title Fix CI: Temporarily pin coloredlogs <= 14.0 Fix CI Dec 31, 2020
@hugovk
Copy link
Contributor Author

hugovk commented Dec 31, 2020

And fix PyPy2 by installing cryptography < 3, a dependency of coveralls.

Version 2.9 was the last version that passed tests, and 3.2.1 the first one that failed tests.

xolox added a commit to xolox/python-coloredlogs that referenced this pull request Jun 11, 2021
The StandardErrorHandler class is responsible for dynamically resolving
(looking up the value of) sys.stderr for each logged message instead of
once when coloredlogs.install() is called.

This was unintentionally broken by changes in 4f8c9aa which were
released as part of coloredlogs version 14.1.

This was brought to my attention by @hugovk here:
xolox/python-humanfriendly#51
@xolox xolox merged commit b99a59e into xolox:master Jun 11, 2021
@xolox
Copy link
Owner

xolox commented Jun 11, 2021

Hi Hugo, thanks for the feedback & analysis. I merged your changes to get cryptography pinned, then I fixed the upstream bug in coloredlogs (including a new regression test) and I've since bumped the test dependency in humanfriendly.

A general note about CI: I'm looking into replacing Travis CI (no longer available for open source projects in any useful capacity, as you undoubtedly know) with e.g. GitHub Actions but have zero experience with the latter, so it may be a while until I have CI infrastructure back up and running...

@hugovk hugovk deleted the fix-ci branch June 11, 2021 12:07
@hugovk
Copy link
Contributor Author

hugovk commented Jun 11, 2021

I have lots of experience with GitHub Actions, I'll make a PR!

@hugovk
Copy link
Contributor Author

hugovk commented Jun 11, 2021

Please see PR #54.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants