Re: [PATCH 3/3] tests: deterministichash: Make hash tables arch-independent

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Aug 03, 2017 at 10:28:59 +0200, Bjoern Walk wrote:
> Peter Krempa <pkrempa@xxxxxxxxxx> [2017-08-03, 09:47AM +0200]:
> > On Thu, Aug 03, 2017 at 07:24:35 +0200, Bjoern Walk wrote:
> > > Peter Krempa <pkrempa@xxxxxxxxxx> [2017-08-02, 05:39PM +0200]:
> > > > It turns out that our implementation of the hashing function is
> > > > endian-dependent and thus if used on various architectures the testsuite
> > > > may have different results. Work this around by mocking virHashCodeGen
> > > > to something which does not use bit operations instead of just setting a
> > > > deterministic seed.
> > > 
> > > This does fix the issue on S390. Anyways, I'd also like to see the tests
> > > fixed that rely on the ordering of the hash table. And code that uses
> > 
> > The tests are fixed. They are ordered correctly to the newly mocked
> > function. I don't quite get what more you'd like to see fixed.
> > 
> 
> No, the test is still dependent on the ordering. Just now the mocked
> hash table has deterministic ordering. This is not the same. Although it
> is improbable that this will bite us somewhere, I'd still prefer a clean
> solution.

I don't think it's worth doing this in the tests. The hash table at
least in case of the node name detection code is just an intermediate
step to fill the data into the domain definition and it's a conveniet
place to test the detection code.

Testing it in other places would remove the dependency on deterministic
ordering of the hash table but would not really add much value. The cost
would be much more complexity in the test case which I don't think it's
worth.

If you feel bothered by it, please post patches. I think currently the
testsuite tests the complex portion of the code without the bloating
necessary for the so-called "clean" solution.

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux