This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by kadircet on Jan 29 2020, 2:18 AM.

Diff Detail

Event Timeline

kadircet created this revision.Jan 29 2020, 2:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 29 2020, 2:18 AM
sammccall accepted this revision.Jan 29 2020, 2:31 AM
sammccall added inline comments.
clang-tools-extra/clangd/unittests/HoverTests.cpp
583

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)

586

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

This revision is now accepted and ready to land.Jan 29 2020, 2:31 AM

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 updated this revision to Diff 241085.Jan 29 2020, 2:59 AM
kadircet marked 4 inline comments as done.
  • Add fixme for delayed-template-parsing
clang-tools-extra/clangd/unittests/HoverTests.cpp
583

right adding a fixme and sending a follow-up patch

586

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 added inline comments.Jan 29 2020, 3:13 AM
clang-tools-extra/clangd/unittests/HoverTests.cpp
586

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.

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.

This revision was automatically updated to reflect the committed changes.