Re: [PATCH v6 25/25] python: add tox support

From: John Snow <jsnow@redhat.com>
To: Cleber Rosa <crosa@redhat.com>
Cc: "Kevin Wolf" <kwolf@redhat.com>, "Fam Zheng" <fam@euphon.net>,
	"Thomas Huth" <thuth@redhat.com>,
	"Eduardo Habkost" <ehabkost@redhat.com>,
	qemu-block@nongnu.org,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	qemu-devel@nongnu.org,
	"Wainer dos Santos Moschetta" <wainersm@redhat.com>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Willian Rampazzo" <willianr@redhat.com>,
	"Willian Rampazzo" <wrampazz@redhat.com>,
	"Max Reitz" <mreitz@redhat.com>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"Beraldo Leal" <bleal@redhat.com>
Subject: Re: [PATCH v6 25/25] python: add tox support
Date: Tue, 25 May 2021 16:25:37 -0400	[thread overview]
Message-ID: <9a50d093-9eea-850c-3d5d-bd44b0f45551@redhat.com> (raw)
In-Reply-To: <YK1ayNF0VTSJKa6O@localhost.localdomain>

On 5/25/21 4:15 PM, Cleber Rosa wrote:
> On Wed, May 12, 2021 at 07:12:41PM -0400, John Snow wrote:
>> This is intended to be a manually run, non-CI script.
>>
>> Use tox to test the linters against all python versions from 3.6 to
>> 3.9. This will only work if you actually have those versions installed
>> locally, but Fedora makes this easy:
>>
>>> sudo dnf install python36 python37 python38 python39
>>
>> Unlike the pipenv tests (make venv-check), this pulls "whichever"
>> versions of the python packages, so they are unpinned and may break as
>> time goes on. In the case that breakages are found, setup.cfg should be
>> amended accordingly to avoid the bad dependant versions, or the code
>> should be amended to work around the issue.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   python/README.rst |  2 ++
>>   python/.gitignore |  1 +
>>   python/Makefile   |  7 ++++++-
>>   python/setup.cfg  |  1 +
>>   python/tox.ini    | 13 +++++++++++++
>>   5 files changed, 23 insertions(+), 1 deletion(-)
>>   create mode 100644 python/tox.ini
>>
> 
> This works as intended for me.  A couple of notes / suggestions
> for future improvements:
> 
>   * `dnf install tox` pulled all the Python versions available (I
>     assume as suggestions) automatically
> 
>   * tox.ini can be folded into setup.cfg
> 

Done!

>   * a custom container image with all those Python versions may be
>     handy for running both the pipenv based job (along with the
>     suggestions on the previous patch) and an on-demand,
>     "allow_failure" tox based CI job.
> 

Yeah, I was thinking this would be good, too!

I think at this point, it's going to be a follow-up, though. Because 
ideally, yes, this SHOULD pass -- it's just that it needs a fairly 
particular environment to run in, which is annoying, so it's here as an 
optional-ish thing for now.

Maybe I'll make a new fedora:latest container that's meant solely for 
testing python stuff, because it's just such a convenient distro for it.

Later, though.

> Other than those suggestions, this LGTM!
> 
> Reviewed-by: Cleber Rosa <crosa@redhat.com>
> Tested-by: Cleber Rosa <crosa@redhat.com>
> 

🎉