[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