<div dir="ltr">Doesn't sound like there's opposition - can't say whether or not it's wort hit, but if you're willing to give it a go/send a patch, probably can't hurt. Might be a bit of work to get all the corners right - I think there are a few local patches to the gtest in llvm, unfortunately, that you'd have to port over, etc.</div><br><div class="gmail_quote"><div dir="ltr">On Thu, Mar 22, 2018 at 3:59 AM James Henderson <<a href="mailto:jh7370.2008@my.bristol.ac.uk">jh7370.2008@my.bristol.ac.uk</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div>Thanks for the comments all. I've thought about it and I agree that I can probably reduce the number of test cases, so that it is not combinatorial, in my use case.<br><br></div>I guess the broader question still is there as to whether a) people would be opposed to updating to a non-official-release version of googletest, and b) whether it would be worthwhile doing so. As I noted previously, there is at least one other place in the tests that said it would like to use Combine, so it still seems like there would be some benefit in it, although I admit that I haven't looked to make sure that this is a reasonable request there.<br><br></div></div><div dir="ltr">James<br></div><div class="gmail_extra"><br><div class="gmail_quote">On 20 March 2018 at 20:30, <span dir="ltr"><<a href="mailto:paul.robinson@sony.com" target="_blank">paul.robinson@sony.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="m_132153074690056262HOEnZb"><div class="m_132153074690056262h5">>> In my particular case from <a href="https://reviews.llvm.org/D44560" rel="noreferrer" target="_blank">https://reviews.llvm.org/D44560</a>, I currently<br>
>> test the following 3 different cases across the full set of DWARF<br>
>> versions and formats:<br>
>> - Parsing a valid line table<br>
>> - Emitting an error if the stated prologue length is greater than the<br>
>> actual length<br>
>> - Emitting an error if the stated prologue length is shorter than the<br>
>> actual length<br>
>> The first is just testing the positive cases for each possible input. I<br>
>> guess a single test for DWARF64 could be written for versions 2-4 and<br>
>> another for version 5 (where there is more stuff between the two length<br>
>> fields so this becomes interesting), similarly for DWARF32. To summarise,<br>
>> I think that these cases are interesting: V5 + 32, V5 + 64, V2 + 32/64,<br>
>> V3 + 32/64, V4 + 32/64. The biggest issue I have with cutting down to<br>
>> just this set is that it makes the tests less specific, e.g. at a glance,<br>
>> what is important about the test - the fact that it is v4, or DWARF64,<br>
>> or both independently, or the combination?<br>
>><br>
>> Related aside: I've realised since earlier that there is scope for<br>
>> version 2 tests, distinct from version 3: v2 tests test the lower<br>
>> boundary on valid versions, and v3 the upper boundary on versions<br>
>> without maximum_operations_per_instruction.<br>
>><br>
>> The latter two test cases are important because a) the length field has<br>
>> a different size for DWARF32/64 and therefore the prologue length needs<br>
>> to be measured from a different point between the different formats, and<br>
>> b) the contents of the prologue are different in each of version 3, 4,<br>
>> and 5, and thus the amount read will be different. We could test each<br>
|<br>
>> individual version, independently of the format, but it is theoretically<br>
>> possible for an error to sneak in whereby the two different failure<br>
>> points cancel each other out. The benefit is admittedly small, but these<br>
>> tests are fast, so I don't think it hurts to have them.<br>
><br>
> Still not sure I follow all of this - though perhaps the test design<br>
> discussion might be better in its own thread. But the broad subject/topic/<br>
> name of the stuff I'm interested in applying here is equivalence<br>
> partitioning: <a href="https://en.wikipedia.org/wiki/Equivalence_partitioning" rel="noreferrer" target="_blank">https://en.wikipedia.org/wiki/Equivalence_partitioning</a> -<br>
> helps explain the general idea of reducing "test all combinations/<br>
> permutations of cases" to "test representative cases from each class".<br>
><br>
> - Dave<br>
<br>
</div></div>Equivalence classes are a fine way to reduce duplication in manually<br>
written tests. They tend to depend on a white-box understanding of the<br>
system-under-test to define the equivalences reasonably. LLVM regression<br>
tests tend to be written very much white-box, as it's the developer who<br>
is writing the test. :-) The problem there is that as the code evolves,<br>
the original equivalence classes may change, but that doesn't necessarily<br>
make any tests fail, it just ends up with reduced coverage. Without good<br>
coverage analysis we can easily find tests no longer verifying everything<br>
we wanted them to.<br>
<br>
(And in fact back at HP I had an interesting chat with a product owner<br>
who was very much a fan of combinatoric testing; he said every single<br>
time he added a new dimension, he found new bugs, and didn't think he<br>
could have predicted which instances would have found the bugs.)<br>
<br>
That said, in the case at hand I think looking at the testing at the<br>
granularity of "parsing a line table" is too coarse. There are several<br>
different bits to it and we can reasonably anticipate that they will<br>
remain relatively independent bits:<br>
- detecting the 32/64 format<br>
- extracting the version-dependent sets of fields accurately<br>
- detecting mismatches between the stated length and the internally<br>
consistent length (e.g., the header says length X, but the file<br>
table has N entries which runs it to length > X).<br>
<br>
To the extent that new DWARF versions have different fields, we should<br>
test that the new fields are extracted and reported (the success cases).<br>
If the new fields are 32/64-sensitive, those should be tested in both<br>
formats to make sure that sensitivity is handled (more success cases).<br>
The notion of running past the stated length (conversely, finishing the<br>
parsing before reaching the stated length) intuitively isn't either<br>
version-sensitive or format-sensitive, i.e. all those cases look like<br>
they should be part of the same equivalence class, so you don't need<br>
to replicate those across all versions and formats. With the caveat<br>
that if white-box examination shows that some case takes a very<br>
different path, it's worth replicating the test. In particular the<br>
v5 directory/file tables are very different from prior versions, so<br>
it's worth having separate bad-length tests for pre-v5 and v5.<br>
<br>
--paulr<br>
</blockquote></div><br></div>
</blockquote></div>