On Wed, Oct 18, 2017 at 04:46:20PM +0200, Paolo Bonzini wrote: > On 17/10/2017 18:31, Jeff Cody wrote: > > ) > > else > > + # Do this in a sub-shell, so we are operating on the right > > + # TEST_DIR / QEMU_TEST_DIR > > ( > > export TEST_DIR=$TEST_DIR_SEQ > > cd "$source_iotests"; > > Where is the missing ")"?
It's part of the diff context, not a change itself... it's still there, just not shown in the patch. > > > @@ -837,6 +841,15 @@ do > > fi > > fi > > > > + # Do this in a sub-shell, so we are operating on the right > > + # TEST_DIR / QEMU_TEST_DIR > > + ( > > + export TEST_DIR=$TEST_DIR_SEQ > > + . "$source_iotests/common.config" > > + . "$source_iotests/common.rc" > > + > > + _cleanup_protocols > > "check" wasn't including common.rc before this patch, and most of the > content of that file doesn't apply to "check". So if we want to move > cleanup code to "check" we should remove it from common.rc too. > > In general, I think we should strive to have a better separation between > tests and harness. This for example could let us write a different > harness for the same tests and integrate the tests better into Avocado. > Hence I see one advantage and one disadvantage in your series: > > * by adding more functionality to "check", it shows that the current > separation may fail with a more sophisticated harness (such as the one > you are creating here) > > * it adds a lot more knowledge of QEMU (especially protocols) in > "check", but there is still some unbalance: tests create the images and > the protocol servers, but the harness cleans it up. The visible result > of this unbalance is for example how multi-process protocol tests can > fail when multiple tests try to bind the same address. > > I think it's the right direction, but it feels like it's not there > yet... Sorry---I know this is not very constructive, but I hope it > helps anyway. > > Maybe we should actually rewrite "check" in Python. That would force us > to think more about the design. > I think writing a more sophisticated harness in another language, such as Python, has merit. But to clarify, do you mean that as a 'nack' to this series, or as something to be done later, after this series? If this series improves the existing harness, I don't see why to exclude it because a new re-write could be superior (which I don't dispute).
