Page MenuHomePhabricator

[clangd][test] Disable a particular testcase in FindExplicitReferencesTest when LLVM_ENABLE_EXPENSIVE_CHECKS
ClosedPublic

Authored by jkorous on Jan 16 2020, 3:04 PM.

Details

Summary

I don't understand the cause but need to stop the job on Green Dragon failing now.

  • TEST 'Clangd Unit Tests :: ./ClangdTests/FindExplicitReferencesTest.All' FAILED ****

Note: Google Test filter = FindExplicitReferencesTest.All
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from FindExplicitReferencesTest
[ RUN ] FindExplicitReferencesTest.All
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-expensive/llvm-project/clang-tools-extra/clangd/unittests/FindTargetTests.cpp:949: Failure

Expected: ExpectedRefs
Which is: "0: targets = {ns1::func, ns2::func}\n1: targets = {t}\n"

To be equal to: Actual.DumpedReferences

Which is: "0: targets = {ns2::func, ns1::func}\n1: targets = {t}\n"

With diff:
@@ -1,2 +1,2 @@
-0: targets = {ns1::func, ns2::func}
+0: targets = {ns2::func, ns1::func}
1: targets = {t}\n

Diff Detail

Event Timeline

jkorous created this revision.Jan 16 2020, 3:04 PM
This revision was not accepted when it landed; it landed in state Needs Review.Jan 16 2020, 3:13 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJan 16 2020, 3:13 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript

kadir has a pending (or landed?) fix for this.

It's just a usual fragile test - it's asserting the order, but findExplicitReferences just returns the order given by targetDecls which isn't stable.

We settled on defining a stable (but arbitrary) order for targetDecl as this pattern may be replicated elsewhere.

Please revert this, it was already fixed by d54d71b67e60

jkorous edited the summary of this revision. (Show Details)Jan 16 2020, 5:27 PM

Hi @jkorous, reverting this as d54d71b67e60 should've fixed this. Please let us know if that's not the case.