On 01/28/2016 03:30 PM, Cole Robinson wrote:
Most qemuxml2xml tests expect that the input XML is unchanged after
parsing. This is unlike 99% of new qemu configs in the wild, which after
initial parsing end up with stable PCI device addresses. The xml2xml bit
doesn't currently hit that code path though, so most XML testing indeed
does not change.
Future patches will add that PCI address bits, which means most test cases
will have different output. So let's do away with the hardcoded same vs
different test split, and always track a separate output file. Tests can
still have same input and output, it just necessitates 2 separate XML files.
---
.../qemuxml2xmlout-aarch64-aavmf-virtio-mmio.xml | 49 +++++
If there's going to be a ton of new files added anyway, this is a good
chance to shorten the names of all the files (and avoid needing to do it
to so many more files later). I've always thought it was pointless to
have a file called
qemuxml2xmloutdata/qemuxml2xmlout-blah.xml
why not just:
qemuxml2xmloutdata/blah.xml
?? I recall there being problems with running make rpm (or maybe make
distcheck?) because some filename in one of these directories was beyond
the tarfile name length limit or something like that.
I went back and read Martin's opinion about adding all these files, and
your response. It's true that we aren't *explicitly* testing the PCI
address assignment directly in the xml2xml tests now. It does end up
being tested in the xml2argvtests though (except for devices that are at
fixed PCI addresses which don't show up on the qemu commandline, e.g.
builtin IDE controller in 440fx or builtin SATA on q35). Of course it
would be a lot easier to see what's going on if the XML files could be
compared, rather than trying to compare qemu commandlines and
backtracking to find the offending device in the XML :-)
Do you know what percentage of these files end up being different
between source and result after all your changes are done? If it's
all/most of them, then I think it would be a lot of extra effort for no
gain to setup symbolic links now only to have them broken a few commits
later.
(A bit of thinking out loud follows...)
Anyway, even once we add the PCI address assignment into the
qemuxml2xmltest, we still may have a considerable number of tests that
have the PCI address in the source xml already anyway. Also, I think too
many of the tests have been created with the following formula:
1) copy testfiles for some random existing test
2) add on a line or two that tests the new feature
This has the upside of testing combinations of items that might not
otherwise be tested, but there's nothing methodical about it, and we're
ending up testing the same bits hundreds of times in exactly the same
way. *AND* (the most important) if something is purposefully changed in
one of those bits that is unnecessarily copied to a couple hundred tests
(e.g. the output now has a PCI address in the element), then that change
must be accompanied by changes to hundreds of test output files.
It might be useful to eliminate a bunch of this duplication of tests.
But of course that would be *very* tedious, and the potential for
accidentally removing a useful and unique combination would be high
(especially if we fell to the temptation of letting a newcomer do such a
trimming as a "starter" project).
(anyway...)
It would be nice if we could move all the tests that are there merely
for checking genericxml2xml over to that test in advance of this change
in order to reduce the churn, but I understand your reluctance to do
that - it will involve a lot more subjective decisions about what was
the purpose of various test cases (and for some of those the only clue
you may have is the name of the data file), so it seems like a large
annoying bookkeeping job when what you really need is to make ARM work
correctly :-)
Since we already have hundreds of test files, and this seems to be
moving in the right direction (although taking a nasty side-step), I'm
inclined to ACK this patch. We should maybe put it to some kind of
informal vote, though, just to make sure everyone's concerns are
properly addressed (and maybe someone will come up with a bright idea)
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list