[PATCH] D18757: [Coverage] Update testing methods to support more than two files.

Vedant Kumar via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 6 09:57:52 PDT 2016


vsk added a comment.

Thanks! Lgtm with a few nits.


================
Comment at: llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp:207
@@ +206,3 @@
+  for (unsigned I = 0; I < N; ++I) {
+    ASSERT_GT(N, OutputCMRs[I].FileID);
+    ASSERT_GT(N, OutputCMRs[I].LineStart);
----------------
ISTM that these asserts could be tightened, e.g `ASSERT_EQ(I, OutputCMRs[I].FileID)`.

================
Comment at: llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp:209
@@ +208,3 @@
+    ASSERT_GT(N, OutputCMRs[I].LineStart);
+    EXPECT_EQ(FileNames[OutputCMRs[I].LineStart],
+              OutputFiles[OutputCMRs[I].FileID]);
----------------
Is there a reason to prefer `OutputCMRs[I].LineStart` over `OutputCMRs[I].FileID` on this line?

================
Comment at: llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp:231
@@ +230,3 @@
+    CoverageData Data = LoadedCoverage->getCoverageForFile(FileNames[I]);
+    ASSERT_LE(1, std::distance(Data.begin(), Data.end()));
+    EXPECT_EQ(I, Data.begin()->Line);
----------------
I think this would be easier to read as `EXPECT_TRUE(!Data.empty())`.


http://reviews.llvm.org/D18757





More information about the llvm-commits mailing list