On 05/29/2018 03:06 PM, Eduardo Habkost wrote: > 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. >
We already have a "probable next steps" planning (see PATCH 0), and we can include a "Python module reorganization" to it. So, I'm dropping this test on v3 and we'll add it later, at a better location. - Cleber.
