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