simark marked an inline comment as done.
simark added inline comments.

================
Comment at: unittests/clangd/ClangdTests.cpp:492
+
+  EXPECT_TRUE(DiagConsumer.contains(FooCpp));
+  EXPECT_TRUE(DiagConsumer.contains(BarCpp));
----------------
simark wrote:
> ilya-biryukov wrote:
> > Maybe expose a copy of the map from DiagConsumer and check for all files in 
> > a single line?
> > 
> > ```
> > class MultipleErrorCHeckingDiagConsumer {
> >    /// Exposes all files consumed by onDiagnosticsReady in an unspecified 
> > order.
> >    /// For each file, a bool value indicates whether the last diagnostics 
> > contained an error.
> >    std::vector<std::pair<Path, bool>> filesWithDiags() const { /* ... */ }
> > };
> > 
> > /// ....
> > EXPECT_THAT(DiagConsumer.filesWithDiags(), 
> > UnorderedElementsAre(Pair(FooCpp, false), Pair(BarCpp, true), Pair(BazCpp, 
> > false));
> > ```
> > 
> > It would make the test more concise.
> Nice :)
It's also better because we don't have to explicitly check that Baz is not 
there.  So if some other file sneaks in the result for some reason and 
shouldn't be there, the test will now fail, where it wouldn't have previously.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D39571



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to