RFC PATCH: Issue 90 (Test Clarification)

[Peter Krempa]

On Tue, Dec 29, 2020 at 10:17:31 -0600, Ryan Gahagan wrote:
> On Tue, Dec 29, 2020 at 10:11 AM Peter Krempa <pkrempa at redhat.com> wrote:
> 
> > Please don't post screenshots of text. It's pointlessly bloating the
> > mail and I can't respond inline. Please re-post all the relevant bits as
> > text.

I've remembered a few other reasons to avoid screenshots. Mostly I can't
copy&paste if I wanted to look for something you've posted and also the
contents of the image are not indexed by search engines.

Also please note that the reviews may seem nitpicky, but I'm trying to
be thorough. By merging your code we'll need to support and maintain the
code in the future even in cases when you decide to no longer
participate in our project, thus we need to avoid as many future
problems as possible.

> Sorry about that, I didn't know what the policy on images was. Basically,
> we used the REGENERATE_OUTPUT for the xml2xmltest and inside the generated
> output there is a line which reads "<backingStore type='network'
> index='1'>". However, the xml2xmltest fails on test 181 with the error:
> Expect [ index='1'>]
> Actual [>]
> 
> This backingStore is the only place in the file with an index='1', so
> logically we tried removing that attribute. However, this causes test 182
> to fail (even though it passed before this change) with the error:
> Expect [>]
> Actual [ index='1'>]
> 
> It seems like these tests are at odds with one another and we aren't sure
> what the cause or fix would be. If we use an index, then 181 fails, and if
> we don't, then 182 will fail. Do you know what might be causing this?

Ah, yes. That test is a bit magic when the definition XML of a live VM
differs from a definition XML of an inactive VM and in such case
VIR_TEST_REGENRATE_OUTPUT=1 will not do the right thing fully
automatically.

You need to create tests/qemuxml2xmloutdata/$TESTNAME-inactive.xml as an
empty file and then re-REGENERATE the output. The presence of the
-inactive file is registered by the test runner and then two distinct
versions are allowed.

Also note that if you provided full text output of the test I would be
able to replace $TESTNAME by the actual file name you need to use, by
copying and pasting.

Also, in this case it wasn't necessary, but always provide at least link
to your repository if you've modified the code. Making reviewers job as
easy as possible is the best way to expedite the review process. Review
is a rather scarse resource in many projects, so having the reviwer
guess or look for additional data wastes the available resource.