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

Python 3.12.9 support #138

Merged
merged 3 commits into from
Mar 2, 2025
Merged

Python 3.12.9 support #138

merged 3 commits into from
Mar 2, 2025

Conversation

TheDSCPL
Copy link
Contributor

@TheDSCPL TheDSCPL commented Mar 2, 2025

Failing test:

    def test_unknown_command(capsys):
        result = main(argv=['unknown', 'a1', 'a2'], commands=dict(fake=FakeCommand))
        assert result == 2
        captured = capsys.readouterr()
        exp = "invalid choice: 'unknown' (choose from 'fake')"
>       assert exp in captured.err
E       assert "invalid choice: 'unknown' (choose from 'fake')" in "usage: python3 -m deal [-h] {fake} ...\npython3 -m deal: error: argument {fake}: invalid choice: 'unknown' (choose from fake)\n"
E        +  where "usage: python3 -m deal [-h] {fake} ...\npython3 -m deal: error: argument {fake}: invalid choice: 'unknown' (choose from fake)\n" = CaptureResult(out='', err="usage: python3 -m deal [-h] {fake} ...\npython3 -m deal: error: argument {fake}: invalid choice: 'unknown' (choose from fake)\n").err

tests/test_cli/test_main.py:27: AssertionError

Even though argparse's documentation specifies that it single-quotes the choices, it actually doesn't, according to their own tests.

Failing test:

```
    def test_unknown_command(capsys):
        result = main(argv=['unknown', 'a1', 'a2'], commands=dict(fake=FakeCommand))
        assert result == 2
        captured = capsys.readouterr()
        exp = "invalid choice: 'unknown' (choose from 'fake')"
>       assert exp in captured.err
E       assert "invalid choice: 'unknown' (choose from 'fake')" in "usage: python3 -m deal [-h] {fake} ...\npython3 -m deal: error: argument {fake}: invalid choice: 'unknown' (choose from fake)\n"
E        +  where "usage: python3 -m deal [-h] {fake} ...\npython3 -m deal: error: argument {fake}: invalid choice: 'unknown' (choose from fake)\n" = CaptureResult(out='', err="usage: python3 -m deal [-h] {fake} ...\npython3 -m deal: error: argument {fake}: invalid choice: 'unknown' (choose from fake)\n").err

tests/test_cli/test_main.py:27: AssertionError
```
@orsinium
Copy link
Member

orsinium commented Mar 2, 2025

Thank you!

The tests used to pass. I assume it has changed in the latest Python release. I've triggered run CI to confirm. We might need to allow both versions in tests.

UPD: Yep, CI for older Python versions expects the version with quotes.

@orsinium
Copy link
Member

orsinium commented Mar 2, 2025

Huh, it's even more interesting: Python 3.12.3 has the quotes and Python 3.12.9 doesn't. I assume, they lost it by accident, bugfix releases shouldn't change user-facing output without a very good reason.

@orsinium orsinium changed the title Fix test_main.py Python 3.12.9 support Mar 2, 2025
@orsinium orsinium merged commit a9714c6 into life4:master Mar 2, 2025
9 checks passed
@orsinium
Copy link
Member

orsinium commented Mar 2, 2025

Thank you :)

@TheDSCPL
Copy link
Contributor Author

You're welcome! I created a PR on cpython as well, which has some comments on this. They also mentioned this is new to 3.12.9, so nice catch! 😄

@TheDSCPL TheDSCPL deleted the patch-1 branch March 12, 2025 16:00
@TheDSCPL
Copy link
Contributor Author

I also mentioned there that changing the output of an error is a breaking change because it breaks tests that expect to fail, such as this one. Maybe they'll change it back to the quoted version, so it could be interesting to stay tuned to that discussion. That's because, even though argparse doesn't quote anymore, optparse still does, so it's a bit inconsistent.

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

Successfully merging this pull request may close these issues.

2 participants