[clangd][Hover] Make tests hermetic by setting target triplet

[kadircet]

Summary

Fixes https://bugs.llvm.org/show_bug.cgi?id=44696

[sammccall]

clang-tools-extra/clangd/unittests/HoverTests.cpp

585–588

Is this redundant now? I think the default value here comes from the triple.
(Maybe want to leave it so we have the most-trivial-possible-change for the branch, but add a FIXME)

588

this is a good fix for the branch.
For trunk, should we do this in TestTU (i.e. for everything?)

[merge_guards_bot]

Unit tests: pass. 62269 tests passed, 0 failed and 827 were skipped.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[kadircet]

• Add fixme for delayed-template-parsing

clang-tools-extra/clangd/unittests/HoverTests.cpp

585–588

right adding a fixme and sending a follow-up patch

588

it depends, as you've noted this will result in not testing some of the platform dependent bits in other tests as well.
e.g. code completion, some code actions behaves differently on templates on windows, we currently don't act on them(apart from putting necessary flags to make behavior similar to what we care). But at least we notice them via breakages in buildbots, if we put that into TestTU we might not notice such differences until users report so.

[sammccall]

clang-tools-extra/clangd/unittests/HoverTests.cpp

588

Currently we fix such issues by slapping -fno-delayed-template-parsing everywhere though. (Or often other people do it for us).

Heh, we could have TestTU grab a CLANGD_EXTRA_FLAGS environment variable, and make it easy to test delayed parsing on linux occasionally.

[merge_guards_bot]

Unit tests: pass. 62269 tests passed, 0 failed and 827 were skipped.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

clang-tools-extra/clangd/unittests/HoverTests.cpp

class Foo {})cpp";
        }},
   };
   for (const auto &Case : Cases) {
     SCOPED_TRACE(Case.Code);
 
     Annotations T(Case.Code);
     TestTU TU = TestTU::withCode(T.code());
     TU.ExtraArgs.push_back("-std=c++17");
+    // FIXME: This is no longer necessary, as the default behavior is no delayed
+    // parsing in the triplet below.
     TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
+    // Types might be different depending on the target triplet, we chose a
+    // fixed one to make sure tests passes on different platform.
+    TU.ExtraArgs.push_back("--target=x86_64-pc-linux-gnu");
     auto AST = TU.build();
 
     auto H = getHover(AST, T.point(), format::getLLVMStyle(), nullptr);
     ASSERT_TRUE(H);
     HoverInfo Expected;
     Expected.SymRange = T.range();
     Case.ExpectedBuilder(Expected);
 
const std::function<void(HoverInfo &)> ExpectedBuilder;
             HI.Name = "foo";
             // FIXME: Handle composite types with decltype with a printing
             // policy.
             HI.Type = "auto (decltype(a)) -> decltype(a)";
             HI.ReturnType = "int";
             HI.Parameters = {
                 {std::string("int"), std::string("x"), llvm::None}};
           }},
+      {
+          R"cpp(// sizeof expr
+          void foo() {
+            (void)[[size^of]](char);
+          })cpp",
+          [](HoverInfo &HI) {
+            HI.Name = "expression";
+            HI.Type = "unsigned long";
+            HI.Value = "1";
+          }},
+      {
+          R"cpp(// alignof expr
+          void foo() {
+            (void)[[align^of]](char);
+          })cpp",
+          [](HoverInfo &HI) {
+            HI.Name = "expression";
+            HI.Type = "unsigned long";
+            HI.Value = "1";
+          }},
   };
 
   // Create a tiny index, so tests above can verify documentation is fetched.
   Symbol IndexSym = func("indexSymbol");
   IndexSym.Documentation = "comment from index";
   SymbolSlab::Builder Symbols;
   Symbols.insert(IndexSym);
   auto Index =
       MemIndex::build(std::move(Symbols).build(), RefSlab(), RelationSlab());
 
   for (const auto &Case : Cases) {
     SCOPED_TRACE(Case.Code);
 
     Annotations T(Case.Code);
     TestTU TU = TestTU::withCode(T.code());
     TU.ExtraArgs.push_back("-std=c++17");
     TU.ExtraArgs.push_back("-Wno-gnu-designator");
+    // Types might be different depending on the target triplet, we chose a
+    // fixed one to make sure tests passes on different platform.
+    TU.ExtraArgs.push_back("--target=x86_64-pc-linux-gnu");
     auto AST = TU.build();
 
     auto H = getHover(AST, T.point(), format::getLLVMStyle(), Index.get());
     ASSERT_TRUE(H);
     HoverInfo Expected;
     Expected.SymRange = T.range();
     Case.ExpectedBuilder(Expected);
 
```)md";
 
   llvm::StringRef ExpectedPlaintext = R"pt(variable foo
 
 Value = val
 
 def)pt";
   EXPECT_EQ(HI.present().asPlainText(), ExpectedPlaintext);
 }
-
-TEST(Hover, ExprTests) {
-  struct {
-    const char *const Code;
-    const std::function<void(HoverInfo &)> ExpectedBuilder;
-  } Cases[] = {
-      {
-          R"cpp(// sizeof expr
-          void foo() {
-            (void)[[size^of]](char);
-          })cpp",
-          [](HoverInfo &HI) {
-            HI.Name = "expression";
-            HI.Type = "unsigned long";
-            HI.Value = "1";
-          }},
-      {
-          R"cpp(// alignof expr
-          void foo() {
-            (void)[[align^of]](char);
-          })cpp",
-          [](HoverInfo &HI) {
-            HI.Name = "expression";
-            HI.Type = "unsigned long";
-            HI.Value = "1";
-          }},
-  };
-  for (const auto &C : Cases) {
-    Annotations T(C.Code);
-    TestTU TU = TestTU::withCode(T.code());
-    auto AST = TU.build();
-    auto H = getHover(AST, T.point(), format::getLLVMStyle(), nullptr);
-    ASSERT_TRUE(H);
-    HoverInfo ExpectedHover;
-    C.ExpectedBuilder(ExpectedHover);
-    // We don't check for Type as it might differ on different platforms.
-    EXPECT_EQ(H->Name, ExpectedHover.Name);
-    EXPECT_EQ(H->Value, ExpectedHover.Value);
-  }
-}
 } // namespace
 } // namespace clangd
 } // namespace clang