<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jan 29, 2016 at 9:15 AM, Xinliang David Li <span dir="ltr"><<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">The iteration is also encoded in the name.<br></blockquote><div><br></div><div>That it tests compression? If we're using a standard compression algorithm, I would expect we would just test the compression algorithm directly. I don't think there's any need to test that the compression algorithm works in any particular use case (test that the contents is compressed).<br><br>The input seems pretty arbitrarily chosen - it's not clear why that provides the right tradeoff between time and coverage, it looks a bit scattershot ("if we put enough stuff through this, perhaps it'll give confidence that it works") - maybe something better suited to ASan Fuzzing rather than a unit test.<br><br>Blanket testing with broad inputs probably isn't suitable for a unit test.<br><br>A single test that demonstrates that the contents is smaller compressed (if there's a non-compressed option) might be worthwhile. Though it looks like this test doesn't even test that - I assume I could remove the compression support from this library & still pass the test... </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888"><br>
David<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
On Fri, Jan 29, 2016 at 9:03 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
> If we're asking the question - why does this need to be 1024? What's the<br>
> specific purpose of the iteration? (why would 1 be too few iterations, for<br>
> example?)<br>
><br>
> On Fri, Jan 29, 2016 at 9:01 AM, David Li via llvm-commits<br>
> <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br>
>><br>
>> davidxl accepted this revision.<br>
>> davidxl added a comment.<br>
>> This revision is now accepted and ready to land.<br>
>><br>
>> lgtm<br>
>><br>
>><br>
>> <a href="http://reviews.llvm.org/D16726" rel="noreferrer" target="_blank">http://reviews.llvm.org/D16726</a><br>
>><br>
>><br>
>><br>
>> _______________________________________________<br>
>> llvm-commits mailing list<br>
>> <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
><br>
><br>
</div></div></blockquote></div><br></div></div>