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 ")"? > @@ -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. Paolo
