Re: [Qemu-devel] [PATCH v2 6/6] iotest 201: new test for qmp nbd-server-remove

[Vladimir Sementsov-Ogievskiy]

15.01.2018 18:05, Eric Blake wrote:
> On 01/15/2018 08:40 AM, Vladimir Sementsov-Ogievskiy wrote:
>
> > > And the list archives show several threads of people complaining that
> > > ./check failing with a diff that merely shows:
> > >
> > > -.....
> > > +..E..
> > didn't see that. usually, for failed iotests I see
> >
> > -.....
> > +..E..
> >
> > + some kind of assert-fail in one of testcases
> Although deciphering the assert-fail is not always trivial, and it is
> still sorely underdocumented on how to manually reproduce the situation
> that got to the stackdump.

hmm, restart test? is it documented for 194?

I don't see an option to run only one testcase, but unittest supports it,
so we just need to add this option to ./check


Is it really not trivial with my test?

example, inject bug:
--- a/tests/qemu-iotests/201
+++ b/tests/qemu-iotests/201
@@ -76,7 +76,7 @@ class TestNbdServerRemove(iotests.QMPTestCase):
         self.assertEqual(filter_qemu_io(qemu_io_output).strip(),
                          "can't open device 
nbd+tcp://localhost:10900/exp: " +
                          "Requested export not available\nserver 
reported: " +
-                         "export 'exp' not present")
+                         "1export 'exp' not present")

     def do_test_connect_after_remove(self, force=None):
         args = ('-r', '-f', 'raw', '-c', 'read 0 512', nbd_uri)



output:

-.......
+FFF....
+======================================================================
+FAIL: test_connect_after_remove_default (__main__.TestNbdServerRemove)
+----------------------------------------------------------------------
+Traceback (most recent call last):
+  File "201", line 89, in test_connect_after_remove_default
+    self.do_test_connect_after_remove()
+  File "201", line 86, in do_test_connect_after_remove
+    self.assertConnectFailed(qemu_io(*args))
+  File "201", line 79, in assertConnectFailed
+    "1export 'exp' not present")
+AssertionError: "can't open device nbd+tcp://localhost:10900/exp: 
Requested export not available\nserver reported: export 'exp' not 
present" != "can't open device nbd+tcp://localhost:10900/exp: Requested 
export not available\nserver reported: 1export 'exp' not present"
+
[...]


- all obvious. We see what happened and where. And the name of the 
broken testcase.


----
I remember the following problems with iotests, but I do not think that 
this is a reason to deprecate unittest and go
in some custom way. Better is to fix them. It's all are problems of our 
unittest wrapping, not of python unittest.

- asserts and prints are not naturally mixed in final output (isn't it a 
reason for never using print in these tests?)
- no progress, to see the output we should wait until test finished
- if qemu crashed, it is hard to understand, in which testcase

so, my point is: use unittest. It is a standard library and common way 
of doing this. And it is already in Qemu iotests.
It gives good organization of test code.

Maybe, a "plain executable python test" is good for complicated tests, 
which are actually cant be called "unit test", but
which are more like "system wide test", when we actually need only one 
testcase, but it needs several pages of code.

> > so we know in which testcase and in which line it was failed.
> >
> > > makes it rather hard to see WHAT test 2 was doing that caused an error
> > > instead of a pass, let alone set up a reproduction scenario on JUST the
> > > failing test.  Yes, a lot of existing iotests use this unittest layout,
> > > and on that grounds, I'm not opposed to adding another one; but test 194
> > > really IS easier to debug when something goes wrong.
> > >
> > > > And there 3 test cases, sharing same setUp. Do not you say that unittest
> > > > becomes
> > > > deprecated in qemu? I think, if we have only one testcase, we may use
> > > > 194-like approach,
> > > > but if we have more, it's better to use unittest.
> > > Yes, I think a nice goal for improved testing is to write more
> > > python-based iotests in the style that uses actual output, and not just
> > > the unittest framework, in the test log.  It's not a hard requirement as
> > > long as no one has converted existing tests, but is food for thought.
> > I think, it doesn't mean that we should not use unittest at all, we just
> > need more output with
> > it.
> Yes, that's also a potentially viable option.


--
Best regards,
Vladimir