Re: [Qemu-devel] [PATCH 4/5] scripts/qemu.py: introduce set_console() method

[Eduardo Habkost <ehabkost@redhat.com>]

On Fri, May 25, 2018 at 12:57:54PM -0400, Cleber Rosa wrote:
> On 05/25/2018 01:47 AM, Fam Zheng wrote:
[...]
> >> +class QEMU(unittest.TestCase):
> > 
> > 'QEMU' is too generic, what is the intended tests to do here? It seems to be
> > more about exercising the set_console() method.
> > 
> 
> Yes, but again, I'm favoring simpler names when the scope is supposed to
> be limited.  The chances of a name clash here are close to none IMO.  I
> don't think this class would be reused elsewhere, or a class with the
> same name would be imported here.
> 
> > Any test should be added to tests/, not scripts/.
> > 
> 
> One of the reasons for this file to be in this patch was to generate
> this exact discussion.  "qemu.py" itself is not a script, but a
> module/library.  Still it lacks the structure to have accompanying tests.
> 
> I was hoping to get away with these tests here, so that they wouldn't be
> thrown away, until "qemu.py", "qmp/*.py" and others are given the status
> and structure of Python modules.
> 
> I can definitely move these to tests/, but how about its relation to the
> other tests living there and its activation?  Should we recognize its
> existence and add a check-* target?

"scripts/test_qemu.py" makes it look like a script that will test
QEMU.  While we don't rearrange the Python modules to follow a
more Pythonic structure, probably moving it to tests/ is the best
option we have.  Maybe tests/python?

Wherever we store the test code, running those tests on
"make check" is a good idea.

But I wouldn't like this to delay the inclusion of this series.
If rearranging the Python modules would make the job easier,
including the test code only later would be a reasonable option,
too.

-- 
Eduardo