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

Turn EncodingWarning into errors and cleanup pytest.ini #4255

Merged

Conversation

Avasam
Copy link
Contributor

@Avasam Avasam commented Mar 4, 2024

Summary of changes

Attempt at supeerseding #4234 (comment) w/o any potential breaking change
Essentially a band-aid for #3810 to make reading failed tests results more bearable.

Pull Request Checklist

@Avasam
Copy link
Contributor Author

Avasam commented Mar 4, 2024

6 EncodingWarnings left with this, which I think is good enough for this PR https://github.com/pypa/setuptools/actions/runs/8148548361/job/22271588677?pr=4255#step:9:1897

@Avasam Avasam force-pushed the EncodingWarning-spam-reduction-pytest.ini branch from 29766ec to bb9e67c Compare March 4, 2024 23:52
@Avasam Avasam marked this pull request as ready for review March 4, 2024 23:53
pytest.ini Outdated Show resolved Hide resolved
@Avasam Avasam changed the title Use pytest.ini to reduce EncodingWarning spam in tests Update pytest.ini for EncodingWarning from external libraries Mar 8, 2024
@Avasam Avasam force-pushed the EncodingWarning-spam-reduction-pytest.ini branch from 2acaf1e to 63dda17 Compare March 8, 2024 22:22
@Avasam Avasam requested a review from abravalheri April 21, 2024 20:19
@abravalheri abravalheri force-pushed the EncodingWarning-spam-reduction-pytest.ini branch from 5462b63 to 888a1f0 Compare April 22, 2024 15:16
@Avasam
Copy link
Contributor Author

Avasam commented Apr 22, 2024

@abravalheri Looks like there's still at least one EncodingWarning left to be able to fully re-enable them as errors.
Although the pytest context seems to hide the true location, which I assume is somewhere in the build_py command (likely in the run method).
Maybe the call to distutils.util.byte_compile ?

@abravalheri
Copy link
Contributor

abravalheri commented Apr 22, 2024

Yeah, a bit of a shame...

It might be the case there is a open()... read() ... exec() sequence somewhere or a ConfigParser.read() somewhere in distutils that fails only if SETUPTOOLS_USE_DISTUTILS=stdlib...

Worst case scenario, I was wondering if there is a way of introducing a pytest.mark.filterwarnings("ignore::DeprecationWarning") decorator conditionally to SETUPTOOLS_USE_DISTUTILS in the complicated function...

@Avasam
Copy link
Contributor Author

Avasam commented Apr 22, 2024

Up to you, looks like the last ignore line I added works. Although it could hide other issues in that module.
We could "default" that one instead of "ignore".

I also didn't originally add a news fragment for this PR, but now that we're preventing more EncodingWarning from reaching user code, that's a beneficial user-facing change that's worth mentioning.

@abravalheri abravalheri force-pushed the EncodingWarning-spam-reduction-pytest.ini branch from c40ddc6 to 98523ac Compare April 22, 2024 22:01
@abravalheri
Copy link
Contributor

I think I understood the root of the problem: pytest.warns seems to reset the filter.
One last attempt of being more specific...

pytest.ini Outdated Show resolved Hide resolved
@abravalheri
Copy link
Contributor

abravalheri commented Apr 22, 2024

Now, that problem with stdlib's distutils is gone, but there seems to be another weird problem with pytest-mypy and pypy 😩: https://github.com/pypa/setuptools/actions/runs/8791484502/job/24126944440

Screenshots

image
image

This part is really weird:

image

Why the FileNotFoundError is not being caught by the try...except block?

@abravalheri abravalheri force-pushed the EncodingWarning-spam-reduction-pytest.ini branch from 24066c6 to c144690 Compare April 22, 2024 23:02
pytest.ini Show resolved Hide resolved
pytest.ini Show resolved Hide resolved
pytest.ini Outdated Show resolved Hide resolved
@Avasam Avasam changed the title Update pytest.ini for EncodingWarning from external libraries Turn EncodingWarning into errors and cleanup pytest.ini Apr 24, 2024
@abravalheri
Copy link
Contributor

abravalheri commented Apr 24, 2024

@Avasam, this is the change for the windows errors:

---
 setuptools/tests/test_windows_wrappers.py | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/setuptools/tests/test_windows_wrappers.py b/setuptools/tests/test_windows_wrappers.py
index 3f321386f..b27268935 100644
--- a/setuptools/tests/test_windows_wrappers.py
+++ b/setuptools/tests/test_windows_wrappers.py
@@ -110,7 +110,11 @@ class TestCLI(WrapperTester):
             'arg5 a\\\\b',
         ]
         proc = subprocess.Popen(
-            cmd, stdout=subprocess.PIPE, stdin=subprocess.PIPE, text=True
+            cmd,
+            stdout=subprocess.PIPE,
+            stdin=subprocess.PIPE,
+            text=True,
+            encoding="utf-8",
         )
         stdout, stderr = proc.communicate('hello\nworld\n')
         actual = stdout.replace('\r\n', '\n')
@@ -143,7 +147,11 @@ class TestCLI(WrapperTester):
             'arg5 a\\\\b',
         ]
         proc = subprocess.Popen(
-            cmd, stdout=subprocess.PIPE, stdin=subprocess.PIPE, text=True
+            cmd,
+            stdout=subprocess.PIPE,
+            stdin=subprocess.PIPE,
+            text=True,
+            encoding="utf-8",
         )
         stdout, stderr = proc.communicate('hello\nworld\n')
         actual = stdout.replace('\r\n', '\n')
@@ -191,6 +199,7 @@ class TestCLI(WrapperTester):
             stdin=subprocess.PIPE,
             stderr=subprocess.STDOUT,
             text=True,
+            encoding="utf-8",
         )
         stdout, stderr = proc.communicate()
         actual = stdout.replace('\r\n', '\n')
@@ -240,6 +249,7 @@ class TestGUI(WrapperTester):
             stdin=subprocess.PIPE,
             stderr=subprocess.STDOUT,
             text=True,
+            encoding="utf-8",
         )
         stdout, stderr = proc.communicate()
         assert not stdout
-- 
2.43.2

@abravalheri
Copy link
Contributor

abravalheri commented Apr 25, 2024

I am fixing the macos CI error in the upstram/main (#4327) then rebasing this then merging.

Thank you vrey much @Avasam.

@abravalheri abravalheri force-pushed the EncodingWarning-spam-reduction-pytest.ini branch from cfeea70 to 22ca7e5 Compare April 25, 2024 13:23
newsfragments/4255.misc.rst Outdated Show resolved Hide resolved
@abravalheri abravalheri merged commit b1fc698 into pypa:main Apr 25, 2024
19 of 21 checks passed
@Avasam Avasam deleted the EncodingWarning-spam-reduction-pytest.ini branch April 25, 2024 17:06
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