[libvirt] [PATCH 3/8] tests: Move data directories into testQemuData

[Peter Krempa]

On Wed, Mar 13, 2019 at 10:18:52 +0100, Andrea Bolognani wrote:
> On Wed, 2019-03-13 at 09:41 +0100, Peter Krempa wrote:
> > On Thu, Mar 07, 2019 at 16:44:32 +0100, Andrea Bolognani wrote:
> > > This removes a little duplication right away, and more
> > > importantly will allow us to perform some interesting
> > > refactoring later on.
> [...]
> > > @@ -47,6 +48,8 @@ testQemuDataInit(testQemuDataPtr data)
> > >      if (qemuTestDriverInit(&data->driver) < 0)
> > >          return -1;
> > >  
> > > +    data->dataDir = abs_srcdir "/qemucapabilitiesdata";
> > > +
> > 
> > I'm not entirely persuaded that you need to pass this constant in via
> > the data structure. Even after the last patch the value is never
> > modified. Could you please elaborate on the refactorings that this will
> > allow?
> 
> I'll admit I kinda copy-pasted commit messages around, so that's
> probably why the one for this patch sounds a bit hyperbolic :)
> 
> Anyway, by the end of the series we'll have to pass dataDir to
> testQemuCapsIterate() in addition to using it to build the path for
> both the input and output file, so it makes sense to store it into
> a variable instead of building it from abs_srcdir plus the directory
> name two or more times.
> 
> As for why it's stored in the structure, I could easily have used a
> global variable instead, but this approach seemed cleaner, especially
> considering that we're already using the structure to store data that
> is similar in scope (the driver).

Well, I agree that we should pre-construct the string rather than
constructing it multiple times, but since every use of that string is
inside the function which does the testing there's no point passing it
in via the structure.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190313/2e149ac5/attachment-0001.sig>