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

[Igor Kudrin via llvm-commits]

ikudrin added inline comments.

================
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);
----------------
vsk wrote:
> ISTM that these asserts could be tightened, e.g `ASSERT_EQ(I, OutputCMRs[I].FileID)`.
IMHO the test shouldn't be more tighter than necessary. This test only checks that all regions are bound to the correct files and not intended to check the exact order of loaded regions as regions could be rearranged by writer.

================
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]);
----------------
vsk wrote:
> Is there a reason to prefer `OutputCMRs[I].LineStart` over `OutputCMRs[I].FileID` on this line?
The same as above, the test shouldn't rely on any specific sorting order of regions.

================
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);
----------------
vsk wrote:
> I think this would be easier to read as `EXPECT_TRUE(!Data.empty())`.
You are right, I'll change it.


http://reviews.llvm.org/D18757