On Mon, Oct 13, 2014 at 04:52:14PM -0400, David Goulet wrote: > Hello again! > > This is a big one but a very cool addition to libotr! A new test suite > that adds both unit and regression tests. > > You can find the code coverage situation here. > > https://coverage.otr.im/ > > Still not 100% but we'll get there eventually ;). > > For review and merge, here is the branch. > > * bug08 > > Humongeous thanks to Julien Voisin that contributed a lot to it! This is > a very important step which will allow libotr to have more and more > solid foundation for further development.
There are 3 commits on this branch. I merged the first. The second, I don't see what bug this is fixing? You say you found it with the test suite; can you show the test that used to fail and now succeeds? The third is the test suite itself. Thanks so much for this! I just have some concerns. When I ran it, it blocked on the privkey test, as it seemed to want to consume a very large amount of data from /dev/random. I went to look at the source, and individual functions have inconsistent indentation (some lines start with 4 spaces when the lines right after start with a hard tab--please ensure hard tabs correspond to 8 spaces throughout, as discussed earlier). But worse, the privkey test (at least; I didn't check the others) does not appear to be initializing either libgcrypt or libotr before using them? Could you clean these up before I merge them? (And perhaps do something about the blocking of the privkey test?) Then going forward, it would be great if every bugfix branch contained first a commit patching the test suite to add a test that triggers the bug in question, and then a commit fixing the bug (and the test suite should then pass). Again, thanks for the great work on the test suite! - Ian _______________________________________________ OTR-dev mailing list OTR-dev@lists.cypherpunks.ca http://lists.cypherpunks.ca/mailman/listinfo/otr-dev
