[PATCH] D28515: [Support] - Introduce zlib::toString(zlib::Status)

Davide Italiano via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 12 19:58:50 PST 2017


davide requested changes to this revision.
davide added inline comments.
This revision now requires changes to proceed.


================
Comment at: unittests/Support/CompressionTest.cpp:70
 
+TEST(CompressionTest, ZlibMessages) {
+  EXPECT_EQ("", zlib::toString(zlib::StatusOK));
----------------
grimar wrote:
> mgorny wrote:
> > To be honest, I don't think this test takes makes a lot of sense since it's pretty much a duplicate of the function itself. As far as I can see, rather than catching bugs it's going to get people frustrated when they change one of the messages.
> That is why I did not add it in inital diff.
> I was and still unsure if we should or should not test string values like here.
What I actually meant is to create test scenarios triggering these errors, not copying the function itself.


https://reviews.llvm.org/D28515





More information about the llvm-commits mailing list