[PATCH] D39571: [clangd] DidChangeConfiguration Notification

Simon Marchi via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 21 04:41:46 PST 2018


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





More information about the cfe-commits mailing list