On Wed, Feb 09, 2022 at 14:28:29 +0100, Michal Prívozník wrote:
On 2/9/22 10:39, Daniel P. Berrangé wrote:
On Tue, Feb 08, 2022 at 12:22:36PM +0100, Michal Privoznik wrote:
In one of my previous commits, I've changed an XPath in virCPUDefParseXML() from "boolean(./counter...)" to "./counter...)". Notice the dangling closing bracket? Well, I didn't back then.
Suggests we have missing test XML data file example to be added somewhere, as detecting parsing errors are the one thing we are pretty good at in unit tests usually.
Actually we do have a test, well, sort of. We have cputest which if ran standalone prints errors onto stderr:
ibvirt.git/_build/tests $ ./cputest TEST: cputest XPath error : Invalid expression ./counter[@name='tsc']) ^ But since everybody resorts to plain 'ninja test' which cleverly discards these errors (storing them in a file that nobody reads is equivalent) it went undetected. On the other hand, making a test fail on nonempty stderr feels wrong. With the old suite we at least saw stderr interleaved with regular output 😞.
Apparently the root cause in virCPUDefParseXML which just ignores this kind of failure. A failure to get a value by XPath is just interpreted as if the corresponding value was not present in the XML no matter what exactly failed and why. Jirka