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 selftests with Pygments >= 2.19.0 #13113

Merged
merged 2 commits into from
Jan 8, 2025

Conversation

The-Compiler
Copy link
Member

@The-Compiler The-Compiler commented Jan 7, 2025

With Pygments 2.19, the Python lexer now emits
Text.Whitespace (rather than Text) tokens after "def", which get highlighted as "bright black".

See pygments/pygments#1905

Fixes #13112

(also fixes the current CI failures, I hope!)

@@ -168,6 +170,9 @@ def color_mapping():

Used by tests which check the actual colors output by pytest.
"""
# https://github.com/pygments/pygments/commit/d24e272894a56a98b1b718d9ac5fabc20124882a
pygments_version = tuple(int(part) for part in pygments.__version__.split(".")[:2])
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit crude, I thought about using packaging.Version instead (pytest already depends on it). Since we only need major/minor, I suppose this is simpler? Happy to adjust if preferred.

Alternatively we could just adjust this without a version check and assume the newest Pygments version?

Copy link
Member

@nicoddemus nicoddemus Jan 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up to you, given these are self tests I'm OK with this solution... if it ever breaks we can just adjust it to something more elaborate with packaging.Version. 👍

@@ -6,6 +6,8 @@
import re
import sys

import pygments
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes the testsuite explicitly depend on pygments - but the tests in test_terminal.py already fail if pygments is not installed, and it's in optional-dependencies.dev in pyproject.toml, so I suppose that's alright?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine to me.

The alternative would be to import-or-skip inside the fixture function.

@@ -168,6 +170,9 @@ def color_mapping():

Used by tests which check the actual colors output by pytest.
"""
# https://github.com/pygments/pygments/commit/d24e272894a56a98b1b718d9ac5fabc20124882a
pygments_version = tuple(int(part) for part in pygments.__version__.split(".")[:2])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's fetch the version from Importlib.metadata.version

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @The-Compiler for tackling this!

@@ -168,6 +170,9 @@ def color_mapping():

Used by tests which check the actual colors output by pytest.
"""
# https://github.com/pygments/pygments/commit/d24e272894a56a98b1b718d9ac5fabc20124882a
pygments_version = tuple(int(part) for part in pygments.__version__.split(".")[:2])
Copy link
Member

@nicoddemus nicoddemus Jan 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up to you, given these are self tests I'm OK with this solution... if it ever breaks we can just adjust it to something more elaborate with packaging.Version. 👍

With Pygments 2.19, the Python lexer now emits
Text.Whitespace (rather than Text) tokens after "def",
which get highlighted as "bright black".

See pygments/pygments#1905
Fixes pytest-dev#13112
@The-Compiler
Copy link
Member Author

Updated to use packaging.Version and importlib.metadata.

@nicoddemus nicoddemus added the backport 8.3.x apply to PRs at any point; backports the changes to the 8.3.x branch label Jan 8, 2025
@nicoddemus
Copy link
Member

Awesome, thanks!

@The-Compiler The-Compiler merged commit bdfc3a9 into pytest-dev:main Jan 8, 2025
28 checks passed
Copy link

patchback bot commented Jan 8, 2025

Backport to 8.3.x: 💚 backport PR created

✅ Backport PR branch: patchback/backports/8.3.x/bdfc3a99bd733f385f150446caef6d5843bb6418/pr-13113

Backported as #13116

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Jan 8, 2025
Fix selftests with Pygments >= 2.19.0

(cherry picked from commit bdfc3a9)
The-Compiler added a commit that referenced this pull request Jan 8, 2025
…dfc3a99bd733f385f150446caef6d5843bb6418/pr-13113

[PR #13113/bdfc3a99 backport][8.3.x] Fix selftests with Pygments >= 2.19.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 8.3.x apply to PRs at any point; backports the changes to the 8.3.x branch bot:chronographer:provided (automation) changelog entry is part of PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pytest's self tests are failing with pygments v2.19.0
4 participants