[PATCH] D73613: [clangd][Hover] Make tests hermetic by setting target triplet

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 29 03:16:22 PST 2020


sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:586
+    // fixed one to make sure tests passes on different platform.
+    TU.ExtraArgs.push_back("--target=x86_64-pc-linux-gnu");
     auto AST = TU.build();
----------------
kadircet wrote:
> sammccall wrote:
> > this is a good fix for the branch.
> > For trunk, should we do this in TestTU (i.e. for everything?)
> 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.
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73613/new/

https://reviews.llvm.org/D73613





More information about the cfe-commits mailing list