[PATCH] D44764: [clangd] Move GTest printers to separate file

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle updated this revision to Diff 139598.
malaperle added a comment.

Use a header for common inclusions for tests


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44764

Files:
  unittests/clangd/ClangdTesting.h
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/ClangdUnitTests.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/CodeCompletionStringsTests.cpp
  unittests/clangd/ContextTests.cpp
  unittests/clangd/FileIndexTests.cpp
  unittests/clangd/FuzzyMatchTests.cpp
  unittests/clangd/HeadersTests.cpp
  unittests/clangd/IndexTests.cpp
  unittests/clangd/JSONExprTests.cpp
  unittests/clangd/Matchers.h
  unittests/clangd/Printers.h
  unittests/clangd/SourceCodeTests.cpp
  unittests/clangd/SymbolCollectorTests.cpp
  unittests/clangd/TUSchedulerTests.cpp
  unittests/clangd/TestFS.cpp
  unittests/clangd/ThreadingTests.cpp
  unittests/clangd/TraceTests.cpp
  unittests/clangd/URITests.cpp
  unittests/clangd/XRefsTests.cpp

Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -7,32 +7,21 @@
 //
 //===--===//
 #include "Annotations.h"
+#include "ClangdTesting.h"
 #include "ClangdUnit.h"
 #include "Compiler.h"
-#include "Matchers.h"
 #include "SyncAPI.h"
 #include "TestFS.h"
 #include "XRefs.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/PCHContainerOperations.h"
 #include "clang/Frontend/Utils.h"
 #include "llvm/Support/Path.h"
-#include "gmock/gmock.h"
-#include "gtest/gtest.h"
 
 namespace clang {
 namespace clangd {
 using namespace llvm;
 
-void PrintTo(const DocumentHighlight , std::ostream *O) {
-  llvm::raw_os_ostream OS(*O);
-  OS << V.range;
-  if (V.kind == DocumentHighlightKind::Read)
-OS << "(r)";
-  if (V.kind == DocumentHighlightKind::Write)
-OS << "(w)";
-}
-
 namespace {
 using testing::ElementsAre;
 using testing::Field;
Index: unittests/clangd/URITests.cpp
===
--- unittests/clangd/URITests.cpp
+++ unittests/clangd/URITests.cpp
@@ -7,10 +7,9 @@
 //
 //===--===//
 
+#include "ClangdTesting.h"
 #include "TestFS.h"
 #include "URI.h"
-#include "gmock/gmock.h"
-#include "gtest/gtest.h"
 
 namespace clang {
 namespace clangd {
Index: unittests/clangd/TraceTests.cpp
===
--- unittests/clangd/TraceTests.cpp
+++ unittests/clangd/TraceTests.cpp
@@ -7,15 +7,14 @@
 //
 //===--===//
 
+#include "ClangdTesting.h"
 #include "Trace.h"
 
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/YAMLParser.h"
-#include "gmock/gmock.h"
-#include "gtest/gtest.h"
 
 namespace clang {
 namespace clangd {
Index: unittests/clangd/ThreadingTests.cpp
===
--- unittests/clangd/ThreadingTests.cpp
+++ unittests/clangd/ThreadingTests.cpp
@@ -7,8 +7,8 @@
 //
 //===--===//
 
+#include "ClangdTesting.h"
 #include "Threading.h"
-#include "gtest/gtest.h"
 #include 
 
 namespace clang {
Index: unittests/clangd/TestFS.cpp
===
--- unittests/clangd/TestFS.cpp
+++ unittests/clangd/TestFS.cpp
@@ -7,8 +7,8 @@
 //
 //===--===//
 #include "TestFS.h"
+#include "ClangdTesting.h"
 #include "llvm/Support/Errc.h"
-#include "gtest/gtest.h"
 
 namespace clang {
 namespace clangd {
Index: unittests/clangd/TUSchedulerTests.cpp
===
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -7,11 +7,10 @@
 //
 //===--===//
 
+#include "ClangdTesting.h"
 #include "Context.h"
 #include "TUScheduler.h"
 #include "TestFS.h"
-#include "gmock/gmock.h"
-#include "gtest/gtest.h"
 #include 
 #include 
 
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -8,6 +8,7 @@
 //===--===//
 
 #include "Annotations.h"
+#include "ClangdTesting.h"
 #include "TestFS.h"
 #include "index/SymbolCollector.h"
 #include "index/SymbolYAML.h"
@@ -21,8 +22,6 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/MemoryBuffer.h"
-#include "gmock/gmock.h"
-#include 

[PATCH] D44764: [clangd] Move GTest printers to separate file

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added a comment.

In https://reviews.llvm.org/D44764#1045682, @ilya-biryukov wrote:

> In https://reviews.llvm.org/D44764#1045648, @simark wrote:
>
> > We could create a file `ClangdTesting.h" which includes everything tested 
> > related (gtest, gmock, printers, syncapi, etc). The convention would be 
> > that test files would just include that.
>
>
> Yeah, sounds good. I would exclude syncapi from the list, though. Not all 
> tests should necessarily depend on `ClangdServer`.
>  @sammccall , @ioeric , @hokein, @bkramer are you on board with the idea to 
> have an umbrella header for gtest+gmock+printers?


Sounds good to me too. I can update the patch once there is an agreement.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44764



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

[PATCH] D44764: [clangd] Move GTest printers to separate file

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added subscribers: sammccall, hokein, bkramer.
ilya-biryukov added a comment.

In https://reviews.llvm.org/D44764#1045648, @simark wrote:

> We could create a file `ClangdTesting.h" which includes everything tested 
> related (gtest, gmock, printers, syncapi, etc). The convention would be that 
> test files would just include that.


Yeah, sounds good. I would exclude syncapi from the list, though. Not all tests 
should necessarily depend on `ClangdServer`.
@sammccall , @ioeric , @hokein, @bkramer are you on board with the idea to have 
an umbrella header for gtest+gmock+printers?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44764



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

[PATCH] D44764: [clangd] Move GTest printers to separate file

[Simon Marchi via Phabricator via cfe-commits]

simark added a comment.

We could create a file `ClangdTesting.h" which includes everything tested 
related (gtest, gmock, printers, syncapi, etc). The convention would be that 
test files would just include that.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44764



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

[PATCH] D44764: [clangd] Move GTest printers to separate file

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM.
I wonder if there's a mechanism to always include the printers when including 
`gtest.h`, but having a convention to always include them seems ok for now.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44764



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

[PATCH] D44764: [clangd] Move GTest printers to separate file

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle created this revision.
Herald added subscribers: cfe-commits, MaskRay, ioeric, jkorous-apple, 
ilya-biryukov, klimek.

This mitigates a possible issue in tests that print objects on failure. It is
possible that there will be two different instantiations of the printer template
for a given type and some tests could end up calling the wrong (default) one.
For example, it was seen in CodeCompleteTests.cpp when printing CompletionItems
that it would use the wrong printer because the default is also instantiated in
ClangdTests.cpp.

With this change, all tests that need to use printers can include the new header
so that this issue does not occur.

Signed-off-by: Marc-Andre Laperle 


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44764

Files:
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/ClangdUnitTests.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/CodeCompletionStringsTests.cpp
  unittests/clangd/ContextTests.cpp
  unittests/clangd/FileIndexTests.cpp
  unittests/clangd/FuzzyMatchTests.cpp
  unittests/clangd/HeadersTests.cpp
  unittests/clangd/IndexTests.cpp
  unittests/clangd/JSONExprTests.cpp
  unittests/clangd/Matchers.h
  unittests/clangd/Printers.h
  unittests/clangd/SourceCodeTests.cpp
  unittests/clangd/SymbolCollectorTests.cpp
  unittests/clangd/TUSchedulerTests.cpp
  unittests/clangd/TestFS.cpp
  unittests/clangd/TraceTests.cpp
  unittests/clangd/URITests.cpp
  unittests/clangd/XRefsTests.cpp

Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -10,6 +10,7 @@
 #include "ClangdUnit.h"
 #include "Compiler.h"
 #include "Matchers.h"
+#include "Printers.h"
 #include "SyncAPI.h"
 #include "TestFS.h"
 #include "XRefs.h"
@@ -24,15 +25,6 @@
 namespace clangd {
 using namespace llvm;
 
-void PrintTo(const DocumentHighlight , std::ostream *O) {
-  llvm::raw_os_ostream OS(*O);
-  OS << V.range;
-  if (V.kind == DocumentHighlightKind::Read)
-OS << "(r)";
-  if (V.kind == DocumentHighlightKind::Write)
-OS << "(w)";
-}
-
 namespace {
 using testing::ElementsAre;
 using testing::Field;
Index: unittests/clangd/URITests.cpp
===
--- unittests/clangd/URITests.cpp
+++ unittests/clangd/URITests.cpp
@@ -7,6 +7,7 @@
 //
 //===--===//
 
+#include "Printers.h"
 #include "TestFS.h"
 #include "URI.h"
 #include "gmock/gmock.h"
Index: unittests/clangd/TraceTests.cpp
===
--- unittests/clangd/TraceTests.cpp
+++ unittests/clangd/TraceTests.cpp
@@ -7,6 +7,7 @@
 //
 //===--===//
 
+#include "Printers.h"
 #include "Trace.h"
 
 #include "llvm/ADT/DenseMap.h"
Index: unittests/clangd/TestFS.cpp
===
--- unittests/clangd/TestFS.cpp
+++ unittests/clangd/TestFS.cpp
@@ -7,6 +7,7 @@
 //
 //===--===//
 #include "TestFS.h"
+#include "Printers.h"
 #include "llvm/Support/Errc.h"
 #include "gtest/gtest.h"
 
Index: unittests/clangd/TUSchedulerTests.cpp
===
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -8,6 +8,7 @@
 //===--===//
 
 #include "Context.h"
+#include "Printers.h"
 #include "TUScheduler.h"
 #include "TestFS.h"
 #include "gmock/gmock.h"
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -8,6 +8,7 @@
 //===--===//
 
 #include "Annotations.h"
+#include "Printers.h"
 #include "TestFS.h"
 #include "index/SymbolCollector.h"
 #include "index/SymbolYAML.h"
Index: unittests/clangd/SourceCodeTests.cpp
===
--- unittests/clangd/SourceCodeTests.cpp
+++ unittests/clangd/SourceCodeTests.cpp
@@ -6,6 +6,7 @@
 // License. See LICENSE.TXT for details.
 //
 //===--===//
+#include "Printers.h"
 #include "SourceCode.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/raw_os_ostream.h"
Index: unittests/clangd/Printers.h
===
--- /dev/null
+++ unittests/clangd/Printers.h
@@ -0,0 +1,77 @@
+//===-- Printers.h --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed