On Fri, May 25, 2018 at 12:27:46PM -0400, Cleber Rosa wrote: > > > On 05/25/2018 01:37 AM, Fam Zheng wrote: > > On Thu, 05/24 20:58, Cleber Rosa wrote: > >> This patch adds a few simple behavior tests for VNC. These tests > >> introduce manipulation of the QEMUMachine arguments, by setting > >> the arguments, instead of adding to the existing ones. > > > > I'm confused by this. The code uses 'add_args', so it does add to the > > arguments, > > no? > > > > And you should be. I changed the code to just add args, and forgot to > update the commit message. If a better example comes up that requires > setting arguments, I'll get back to this. > > >> > >> Signed-off-by: Cleber Rosa <cr...@redhat.com> > >> --- > >> tests/acceptance/test_vnc.py | 50 ++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 50 insertions(+) > >> create mode 100644 tests/acceptance/test_vnc.py > >> > >> diff --git a/tests/acceptance/test_vnc.py b/tests/acceptance/test_vnc.py > >> new file mode 100644 > >> index 0000000000..9d9a35cf55 > >> --- /dev/null > >> +++ b/tests/acceptance/test_vnc.py > >> @@ -0,0 +1,50 @@ > > > > Copyright header is missing here too. > > > > Indeed. > > > Fam > > > >> +from avocado_qemu import Test > >> + > >> + > >> +class Vnc(Test): > > > > Should VncTest be a better class name? > > > > I'm favoring simpler names. If you think about the complete test names, > it's already too verbose IMO: "test_vnc.Vnc.test_no_vnc". > > That's actually an interesting point: how would you feel about dropping > the "test_" prefix from the file names?
I would like this. The file is already inside a tests/acceptance directory. About the class name, I would be less surprised when reading the code if it was called "VncTest", but I won't object to "Vnc" if you prefer it. -- Eduardo
