Open kloczek opened 1 year ago
Bachmann1234
commented
1 year ago So the most likely reason the suite is failing is that the command is not available on your path. Diff quality runs its commands via subprocess (since it does not assume the command are installed in the same python environment or are even python tools).
This error actually comes up on the command that checks for the tools existence.
kloczek
commented
7 months ago So the most likely reason the suite is failing is that the command is not available on your path. Diff quality runs its commands via subprocess (since it does not assume the command are installed in the same python environment or are even python tools).
This error actually comes up on the command that checks for the tools existence.
OK but which one command? 🤔
In mean time I've moved to python 3.9 and pytest 8.1.1 and now pytest fails in new units
Bachmann1234
commented
7 months ago @kloczek for pylint the command is pylint --version
Based on the output that command is failing (returning a non zero code)
Bachmann1234
commented
7 months ago To elaborate further. It really looks like whatever environment this is running in does not have pylint in its path
kloczek
commented
7 months ago Just tested new 9.0.0 with installed pylint and after that step indeed some units stopped failing however still have one failing unit
Error message does no match to actual situation because in build env is installed flake8
[tkloczko@pers-jacek SPECS]$ pip show flake8
Name: flake8
Version: 6.1.0
Summary: the modular source code checker: pep8 pyflakes and co
Home-page: https://github.com/pycqa/flake8
Author: Tarek Ziade
Author-email: tarek@ziade.org
License: MIT
Location: /usr/lib/python3.10/site-packages
Requires: mccabe, pycodestyle, pyflakes
Required-by: pytest-flake8Just tested new 9.0.0 with installed
pylintand after that step indeed some units stopped failing however still have one failing unitHere is pytest output: Error message does no match to actual situation because in build env is installed
flake8[tkloczko@pers-jacek SPECS]$ pip show flake8 Name: flake8 Version: 6.1.0 Summary: the modular source code checker: pep8 pyflakes and co Home-page: https://github.com/pycqa/flake8 Author: Tarek Ziade Author-email: tarek@ziade.org License: MIT Location: /usr/lib/python3.10/site-packages Requires: mccabe, pycodestyle, pyflakes Required-by: pytest-flake8
I dont really know what to tell you. That test failing implies the command flake8 --version is returning a non zero exit code. The most common reason for this is the command is not actually in the path. SO the library being installed is part of it but ultimately in whatever python running the test is having trouble running that check.
kloczek
commented
7 months ago I dont really know what to tell you. That test failing implies the command
flake8 --versionis returning a non zero exit code. The most common reason for this is the command is not actually in the path. SO the library being installed is part of it but ultimately in whatever python running the test is having trouble running that check.
[tkloczko@pers-jacek build]$ flake8 --version
6.1.0 (mccabe: 0.7.0, pycodestyle: 2.11.0, pyflakes: 3.1.0) CPython 3.10.14 on Linux
[tkloczko@pers-jacek build]$ echo $?
0Problems happens .. you don't need ot say anything 😄
Do you have any propositions about what I can try to do to diagnose this effect? 🤔 FYI: temporary I've added to --deselect list this unit assuming that more likely with testing procedure than tested code.
Bachmann1234
commented
6 months ago I'll take another stab at reproducing this sometime this week. But at this point I think the only path would be putting a line in to log the exception and see how the command is failing
Bachmann1234
commented
6 months ago @kloczek what does which flake8 show? IS that path accessible in whatever you are doing to run the tests?
Because im not convinced python3 -sBm build -w --no-isolation is installing dev dependencies.
I created a virtual env
installed build and poetry
then ran python3 -sBm build -w --no-isolation
❯ flake8
zsh: command not found: flake8This is the output of pip --freeze
build==1.2.1
CacheControl==0.14.0
certifi==2024.2.2
cffi==1.16.0
charset-normalizer==3.3.2
cleo==2.1.0
crashtest==0.4.1
distlib==0.3.8
dulwich==0.21.7
fastjsonschema==2.19.1
filelock==3.13.4
idna==3.7
installer==0.7.0
jaraco.classes==3.4.0
keyring==24.3.1
more-itertools==10.2.0
msgpack==1.0.8
packaging==24.0
pexpect==4.9.0
pkginfo==1.10.0
platformdirs==4.2.0
poetry==1.8.2
poetry-core==1.9.0
poetry-plugin-export==1.7.1
ptyprocess==0.7.0
pycparser==2.22
pyproject_hooks==1.0.0
rapidfuzz==3.8.1
requests==2.31.0
requests-toolbelt==1.0.0
shellingham==1.5.4
tomlkit==0.12.4
trove-classifiers==2024.4.10
urllib3==2.2.1
virtualenv==20.25.2
xattr==1.1.0@kloczek what does
which flake8show? IS that path accessible in whatever you are doing to run the tests?
I've already provided that details opening ticket
flake8 6.1.0Because im not convinced
python3 -sBm build -w --no-isolationis installing dev dependencies.
Of course it is not installing anything because that command is responsible for only build .whl archive. Nevertheless in this case it is issue not with building but with testing.
Bachmann1234
commented
6 months ago @kloczek I don't see anything about the output of which flake8
I see that the library seems to be installed (and I admit last night I neglected to expand the details)
But it still may be interesting to know where in the build system that command is.
What procedure was used to install the quality tools? Simple pip install? Or something else?
kloczek
commented
6 months ago But it still may be interesting to know where in the build system that command is.
Hmm .. what you mean "where"? 🤔
What procedure was used to install the quality tools? Simple pip install? Or something else?
Full build, install and testing procedure is described on top of this ticket 🙄 😋
I'm installing all modules from rpm packages
I'm not interested to test anything what is provided as .whl archives on pypi but only distribution resources on which I'm working.
This is why build is performed with --no-isolation.
Bachmann1234
commented
6 months ago But it still may be interesting to know where in the build system that command is.
Hmm .. what you mean "where"? 🤔
What procedure was used to install the quality tools? Simple pip install? Or something else?
Full build, install and testing procedure is described on top of this ticket 🙄 😋 I'm installing all modules from rpm packages I'm not interested to test anything what is provided as .whl archives on pypi but only distribution resources on which I'm working. This is why build is performed with
--no-isolation.
The path! Running which will confirm the path of the command! So many times in Python builds executables end up in weird places
kloczek
commented
6 months ago The path! Running
whichwill confirm the path of the command! So many times in Python builds executables end up in weird places
I'm building all packages in dedicated build envs created to build only one package. Inside build are installed only build dependencies. In distribution is only one python package which provides /usr/bin/python3 and it is ATM python 3.11.14. After finis build procedure build env is deleted. In other words .. in such env thee is no possibility to have multiple python binaries/versions.
Bachmann1234
commented
6 months ago Clearly I don't know a lot about Linux packaging beyond basic user commands
But the test that's failing is running a command, not running a function that's imported from a lib. Your single package might be combining all the dependencies but it probably is not installing all the commands for the quality tools into some path that can be executed. But again, I know you are able to run it yourself.
So if your package does not have access to run the command then it will fail that test. This is why I keep asking you to run which to try and confirm where that command is. Because you have demonstrated that you can run it. So it's clearly in your users $PATH
But if that command exists in a place where in the context of the test it does not have access then it won't find the executable.
So that's why I keep asking. What does which pylint tell you. Because if pylint is somewhere the command does not see then it might as well not exist at all.
But honestly, I don't think it's likely that seeing where that command is will make it obvious what's going on. I suspect I'm gonna have to expose the underlying error so you can see specifically what's failing. May not be able to do that for a couple weeks though.
I just think the most likely cause is some kind of path error. In the context of the test, it can't find the pylint executable. I think when I expose the error it's going to be something boring like "command not found" and the question will become "but why?!"
kloczek
commented
6 months ago Clearly I don't know a lot about Linux packaging beyond basic user commands
reported issue has nothing to do with packaging so you don't need to know anything about that.
So that's why I keep asking. What does which pylint tell you. Because if pylint is somewhere the command does not see then it might as well not exist at all.
Please have look on provided output .. first time ever and/or one more time.
Error message has nothing to do with pylint.
Why at all test using unis custom code checking "is module XXX installed" if for that can be used OOTB pytest calls? 🤔
Again flake8 is installed in build env and it works in casesof other modules build procedures
[tkloczko@pers-jacek SPECS]$ grep "BuildRequires:.*python3dist(flake8)" *
crypto-policies.spec:BuildRequires: python3dist(flake8)
python-diff-cover.spec:BuildRequires: python3dist(flake8)
python-flake8-bugbear.spec:BuildRequires: python3dist(flake8) >= 3.0.0
python-flake8-dunder-all.spec:BuildRequires: python3dist(flake8)
python-flake8-pyi.spec:BuildRequires: python3dist(flake8) >= 3.2.1
python-mypy.spec:BuildRequires: python3dist(flake8)
python-pytest-flake8.spec:BuildRequires: python3dist(flake8) >= 3.5As you see in above is literally flake8 is listed in case of this module. With installed that module in build it is not possible to start build. pip show that flake8 is installed but code of that unit for some reason is not able to find it.
Bachmann1234
commented
6 months ago I swear im not trying to be difficult here. I know my mixing up of the specific quality tool is not helping. Ive been responding in the morning and at night while juggling other things and thats my bad. But the theory is the same.
Here is a screenshot from your output
The test is an integration test, it executes flake8 --version as a subprocess command and if that command fails it catches the failure and returns that output 'flake8 is not installed.
Now I know you can run this command yourself, you demonstrated it. And I know flake8 is technically installed. But the point is the subprocess is failing to run it one way or another.
I suspect thats because the library is being installed but the executable for said library is not accessable when running the test and I dont know why.
So you have two options to figure this out.
stdin, stdout = process.communicate()
print(stdin)
print(stdout)that should tell you exactly why the subprocess command is failing. Though again, I suspect its gonna be a path failure of some kind. Where it says the command does not exist despite being installed.
kloczek
commented
6 months ago I've modified my spec file %check to:
%check
echo; pip show flake8; echo
%pytest tests/test_violations_reporter.py::TestFlake8QualityReporterTest::test_file_does_not_exist
%pytest %{!?with_failing_tests: \
--deselect tests/test_violations_reporter.py::TestFlake8QualityReporterTest::test_file_does_not_exist \
}and added pip to build dependencies (normally it is not needed) I've send that spec as test build request (each build is performed in build env created from scratch and after build such build env is deleted).
Here is result literally taken from build log of the rpm %check (test suite execution):
As you see pip shows tat flake8 is present and your cede that it is not. Can you explain that? 🤔
Just humble question one more time: why at all this module test procedure needs to check is flake8 installed? 🤔
flake8 as many other modules never should be executed as module over python -m foo because when pyton is executed with -m it adds current directory to sys.path which messes with many such modules.
This is why flake, pytest and many more provides wrapper script like
[tkloczko@pers-jacek SPECS]$ cat /usr/bin/flake8
#!/usr/bin/python3
# -*- coding: utf-8 -*-
import re
import sys
from flake8.main.cli import main
if __name__ == "__main__":
sys.argv[0] = re.sub(r"(-script\.pyw|\.exe)?$", "", sys.argv[0])
sys.exit(main())In other words if on testing is needed flake8 it shoulder be blindly executed as command and not checked as module.
Am I right or not? 🤔
Bachmann1234
commented
6 months ago Just humble question one more time: why at all this module test procedure needs to check is flake8 installed? I mean we are talking about code that was written potentially over a decade ago... and I dont even remember what parts were originally me.... but if im remembering correctly the idea is that diffcover/diffquality does not assume its bundled with anything. All it cares about is the tool it needs to run can be run over the code. So the python that runs diff cover does not need to be the same python that runs flake8 or any of the related tools (hell, some of the tools are not even python tools). so im not confident the python -m foo procedure makes sense in this context.
If this tool was written today it may have been written differently but who knows!.
The check to see if its installed is just running a simple version of the command. Feel free to remove the install check. I dont think it will help you because the main command will fail... likely for the same cause.
As you see pip shows tat flake8 is present and your cede that it is not. Can you explain that? I feel like ive tried to explain it. The command is failing, for some reason or another. If the basic --version command fails the code assumes its not installed and throws an os error and says its not installed. The error message is likely misleading. Ive told you what you could do to get a more specific error message but that would involve editing the code to see whats going on.
In other words if on testing is needed flake8 it shoulder be blindly executed as command and not checked as module.
Am I right or not? 🤔Again, the "check" being run is blindly executing flake8. Executing flake8 is failing and the error the code throws says its not installed. But clearly the language is misleading/wrong because clearly its installed. But for some reason the subprocess command is failing.
kloczek
commented
6 months ago I mean we are talking about code that was written potentially over a decade ago... and I dont even remember what parts were originally me.
Please don't get me wrong. I'm not trying to blame/accuse anything or anyone. I'm only b*dy messenger 😋 "Shit happens .. sometimes" 😄
I feel like ive tried to explain it. The command is failing, for some reason or another. If the basic --version command fails the code assumes its not installed and throws an os error and says its not installed.
Just made another test
+ strace -e trace=execve,fork /usr/bin/pytest -ra -m 'not network' tests/test_violations_reporter.py::TestFlake8QualityReporterTest::test_file_does_not_exist
execve("/usr/bin/pytest", ["/usr/bin/pytest", "-ra", "-m", "not network", "tests/test_violations_reporter.p"...], 0x7fff283d7630 /* 63 vars */) = 0
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=35604, si_uid=1000, si_status=128, si_utime=0, si_stime=0} ---
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=35605, si_uid=1000, si_status=255, si_utime=0, si_stime=0} ---
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=35606, si_uid=1000, si_status=0, si_utime=0, si_stime=0} ---
==================================================================================== test session starts ====================================================================================
platform linux -- Python 3.10.14, pytest-8.1.1, pluggy-1.4.0
benchmark: 4.0.0 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
rootdir: /home/tkloczko/rpmbuild/BUILD/diff_cover-9.0.0
configfile: pyproject.toml
plugins: forked-1.6.0, hypothesis-6.100.0, mock-3.14.0, benchmark-4.0.0, timeout-2.3.1, flaky-3.8.1, subprocess-1.5.0, flake8-1.1.1, datadir-1.5.0
collected 1 item
tests/test_violations_reporter.py --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=33582, si_uid=1000, si_status=1, si_utime=13 /* 0.13 s */, si_stime=2 /* 0.02 s */} ---
F [100%]
========================================================================================= FAILURES ==========================================================================================
__________________________________________________________________ TestFlake8QualityReporterTest.test_file_does_not_exist ___________________________________________________________________
self = <tests.test_violations_reporter.TestFlake8QualityReporterTest object at 0x7fb3bbd00940>
@pytest.mark.disable_all_files_exist
def test_file_does_not_exist(self):
quality = QualityReporter(flake8_driver)
file_paths = ["ajshdjlasdhajksdh.py"]
# Expect that we get no results because that file does not exist
for path in file_paths:
> result = quality.violations(path)
tests/test_violations_reporter.py:1109:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
self = <diff_cover.violationsreporters.base.QualityReporter object at 0x7fb3bbd009a0>, src_path = 'ajshdjlasdhajksdh.py'
def violations(self, src_path):
"""
Return a list of Violations recorded in `src_path`.
"""
if not any(src_path.endswith(ext) for ext in self.driver.supported_extensions):
return []
if src_path not in self.violations_dict:
if self.reports:
self.violations_dict = self.driver.parse_reports(self.reports)
else:
if self.driver_tool_installed is None:
self.driver_tool_installed = self.driver.installed()
if not self.driver_tool_installed:
> raise OSError(f"{self.driver.name} is not installed")
E OSError: flake8 is not installed
diff_cover/violationsreporters/base.py:160: OSError
================================================================================== short test summary info ==================================================================================
FAILED tests/test_violations_reporter.py::TestFlake8QualityReporterTest::test_file_does_not_exist - OSError: flake8 is not installed
===================================================================================== 1 failed in 1.61s =====================================================================================
+++ exited with 1 +++As you see strace does not show any traces of execve() after pytest execution calls 🤔
kloczek
commented
6 months ago Corrected test. I forgot about -f in strace params (trace in forked processes.
Now it shows:
No traces of executing flake8 and why this unit is trying to execute git, hg and file is another mystery 😋 🙄
kloczek
commented
6 months ago Ops .. sorry ..
[pid 36149] execve("/usr/bin/flake8", ["flake8", "--version"], 0x55f6303c2c70 /* 64 vars */) = 0but this execution returns 0 exit code 🤔
Bachmann1234
commented
6 months ago git, hg, and file could be any of a bunch of setup and fixture stuff...
Now the flake8 --version call returning 0 is very interesting.... and surprising
So we know flake8 is a regex based driver (so no custom stuff going on)
This is the function being called there
That function is pretty simple.. if the process return code is 0 then I feel like that must mean this branch got hit, otherwise it would have returned process.returncode would have returned 0
But ill try and find some time tonight to look closer at this test to verify my assumptions about how this traces out are correct.
kloczek
commented
6 months ago Am I right thar you are suggesting that to run this test it needs to b installed some flake8 extension? 🤔
My questions about why at all this needs to be executed seem still is valid. What do you think about that? 🙄
Bachmann1234
commented
6 months ago you should not need anything thats not already defined in the toml. You can see githubs action run this build regularly.
flake8 is executed because this is an integration test and mocks would make assumptions about how the tool runs that could be changed over time as versions shift.
If you really wanna dig into whats happening here I suggest editing the lines I suggested in the installed lib and running the test. That way you can see if there is a better error
kloczek
commented
6 months ago Again .. executed flake8 script returns 0 status.
Bachmann1234
commented
6 months ago Again .. executed
flake8script returns 0 status.
I understand that block of code is either hitting a filenotfound exception or something else is happening.
Whatever is happening is in that block of code and the only way I know to move forward on this is to print stuff in there and see what comes out
Bachmann1234
commented
6 months ago Because despite what your test did that function returned 1 at least that's what the test output is implying
Bachmann1234
commented
6 months ago Because despite what your test did that function returned 1 at least that's what the test output is implying
kloczek
commented
6 months ago If you have any propositions of some diagnostics please let mi know.
Bachmann1234
commented
6 months ago If you have any propositions of some diagnostics please let mi know.
Earlier I suggested sticking a couple of print lines in that function. So that and remove the catch of the filenotfound exception.
That should print out any output from running the command (if any) and if the exception being thrown (which at this point seems more likely)maybe it will provide more context on why.
kloczek
commented
6 months ago Honestly .. I have no idea where I can put those lines 😞
Bachmann1234
commented
6 months ago I traced though the code path earlier
Take this function
And replace it with this version
def run_command_for_code(command):
"""
Returns command's exit code.
"""
process = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
stdout, stderr = process.communicate()
print(stdin)
print(stout)
return process.returncodeSee if the failure output is more interesting
ancieg
commented
1 week ago This is because .flake8 config file is ill-formed - it contains inline comments (see: https://flake8.pycqa.org/en/latest/user/configuration.html).
I suggest to fix the config via the following patch:
diff --git a/.flake8 b/.flake8
index 2ef7b1b..e934134 100644
--- a/.flake8
+++ b/.flake8
@@ -1,13 +1,18 @@
[flake8]
max-line-length=100
select=
- C901, # flake8-mccabe
- E, # flake8-pycodestyle
- F, # flake8-pyflakes
- W, # flake8-pycodestyle
+ # flake8-mccabe
+ C901,
+ # flake8-pycodestyle
+ E,
+ # flake8-pyflakes
+ F,
+ # flake8-pycodestyle
+ W,
ignore =
- W503,E203, # conflict with black formatter
+ # conflict with black formatter
+ W503,E203,
per-file-ignores =
# supression for __init__
diff_cover/tests/*: E501
- diff_cover/tests/fixtures/*: E,F,W
\ No newline at end of file
+ diff_cover/tests/fixtures/*: E,F,WThis is because
.flake8config file is ill-formed - it contains inline comments (see: https://flake8.pycqa.org/en/latest/user/configuration.html). I suggest to fix the config via the following patch:
Following the recommended settings for Python’s configparser, Flake8 does not support inline comments for any of the keys. So while this is fine:
[flake8]
per-file-ignores =
# imported but unused
__init__.py: F401this is not:
[flake8]
per-file-ignores =
__init__.py: F401 # imported but unusedWhy nor switch from flake8 to ruff? 🤔
ancieg
commented
1 week ago Why nor switch from flake8 to ruff? 🤔
I think you should open new issue with this question.
Bachmann1234
commented
1 week ago I'll take a look at this in the next few days when things calm down for me.
As for switching to ruff. I'm not against it. But it's not high on my list of priorities. I hear it's great though
Bachmann1234
commented
1 week ago https://github.com/Bachmann1234/diff_cover/pull/423 should fix this so next time I do a release (Im not confident this change merits one) it should be addressed.
I'm packaging your module as an rpm package so I'm using the typical PEP517 based build, install and test cycle used on building packages from non-root account.
python3 -sBm build -w --no-isolationbuildwith--no-isolationI'm using during all processes only locally installed modulescut off from access to the public network(pytest is executed with-m "not network")pytest is failing with message that that
flake8is not installed despite fact that it is installed. First thing is why test suite is checking is that module installed? 🤔 Here is pytest output:Here is list of installed modules in build env