On Tue, Mar 19, 2024 at 21:31:52 +0100, Michal Prívozník wrote: > On 1/12/24 17:05, Peter Krempa wrote: > > The main goal of part 3 is to add testing based on parsing of the > > libvirt-formatted files from tests/qemuxml2xmloutdata and formatting > > them back, and checking that they are identical. > > Firstly, sorry for resurrecting an old thread. > Secondly, sorry for hijacking it. BUT. > > There were patches sent to the list recently [1] and I realized, the way > we usually used to do things is disturbed. I mean, whenever new device, > device knob, ... was introduced the patch series consisted (roughly) of > the following patches: > > 1) config, XML parser & formatter, RNG, docs AND xml2xml test case, > 2) qemu caps > 3) qemu cmd line AND xml2argv test case. > > Now, since we have this one huge qemuxmlconftest.c it's not as easy. If > I try to introduce a test case in 1), the test case fails as there's no > corresponding .args file. You can add a corresponding .args file that will not yet have the commandline bits added by the patches, as the implementation is missing. This means that the corresponding patch adding the qemu code will need to contain the corresponding .args change. > Fair, but also not fair - the feature is not > finished at the time of 1) so .args shouldn't even be considered. Well, it still covers what the code does at that point, so at the very least in theory it makes sense. In practice, it possibly looks a bit awkward, but the tests can still be added in a separate commit after both the XML and qemu bits are implemented. As long as the patches test the code I don't really have a problem with that. > But if > a test case is added at step 3) - well, then anybody backporting 1) > won't get the xml2xml test case. Pity. I mean, that's the whole point we > split the change into "frontend" and "backend", right? This case would be a problem only if a patchset would add implementations in two hypervisor drivers at same time and you'd decide to backport only the non-qemu one. In such case the qemu XML coverage would be missing. Reallistically that won't happen, but if it did, the corresponding patches to the other hypervisor driver should add their own tests which you really should bacport or you can also add genericxml2xmltest cases. In quite a lot of cases the qemuxml2xmltest case was also added in a separate commit which is totally fine and the 'backport' case above would not cover that one either. > Okay, you may add step 4), which introduces new qemuxmlconftest.c test > case. But it bundles both xml2xml AND xml2argv steps rendering it yet > again unsuitable for backport. I don't see how that would be unsuitable for backport. If you are backporting a qemu feature, then the qemu* tests are relevant. Otherwise you should be backporting the corresponding tests at least for that hypervisor you are adding. > One way out might be to add new testcases to genericxml2xmltest.c, but > somehow that feels wrong. Why? Genericxml2xml test is there for all generic configs. It's just that most code usually adds a qemu implementation and it's both more convenient and sufficient to excercise all things to add just the qemuxmlconftest case. P.S: 37% qemuxml2argv test cases did not have a corresponding qemuxml2xmltest case at the time when I've converted it over. Additional few were actually testing something else due to a mismatch in capabilities. _______________________________________________ Devel mailing list -- devel@xxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
