fix(c,cpp): a file in a unittests/ or foo_unittest/ folder is a test

Merged
CO
8d ago
+204-7

What

isTestPath (src/search/query-utils.ts) recognized tests/, test/, __tests__/, spec/, e2e/, CamelCase source sets like jvmTest/, and foo-test/ / foo_testing/ modules. It did not recognize the folders LLVM, Clang, clangd, lldb, offload and Breakpad keep their unit tests in (unittests/), or glog's foo_unittest/ test programs:

  • /unittests/ contains tests/ only after a t;
  • the [\w.]+[-_]test(?:s|ing)? form needs a separator right before test.

So a helper there with no test-like name read as production code. Examples: LLVM's llvm/unittests/ADT/CountCopyAndMove.h and llvm/unittests/CodeGen/SelectionDAGTestBase.h, Breakpad's src/client/windows/unittests/dump_analysis.cc, glog's src/includes_unittest/glog_includes_logging.cc.

  • isTestPath now also matches a unittests/ folder, and a foo_unittest/ or foo-unittests/ one ((?:^|\/)(?:unittests|[\w.]+[-_]unittests?)\/).
    • A bare unittest/ stays production. CPython's Lib/unittest/ is the framework, and LLVM vendors googletest under third-party/unittest/.
    • Prometheus's cmd/promtool/unittest.go is a file, untouched here; #2434 already keeps it production.
  • isTestSuitePath (src/resolution/name-matcher.ts) counts the same folders as suites, as it counts tests/.

Built on #2434 (the foo_unittest.cc file-name rule, merged as 1fb8691d) and rebased onto main 1ce1167e.

The suite decision

unittests/ counts as a suite:

  • its files build into unit-test binaries and never link into the program;
  • glog builds each foo_unittest/ folder as a test program of its own (ctest --build-and-test);
  • a foo-test/ module stays out of the suite rule, as shipped test support.

There's a C/C++-specific reason this matters. The suite rule's CamelCase file names (FooTest.java, BarTests.swift, …) skip .cpp. On main, none of LLVM's *Test.cpp files were suites, so production locals named Ctx, KV, V or OpInfo linked to what a unit test declares for itself. The suite rule moves 728 such files in llvm/, 286 in clang/, 244 in lldb/ and 103 in clangd.

A middle arm (the isTestPath rule alone) isolates each half. The suite rule alone removed 3,625 edges across the five LLVM samples. All were wrong except lldb's 60 SWIG stand-ins (below), so it is in.

Validation (main 1ce1167e, three arms: base / isTestPath only / both)

Each edge is keyed by its site (kind, source, line, col, refName); every removed, retargeted and added edge was bucketed by how the arms classify its source and target files, then checked by reading the code.

LLVM samples are subtrees of llvm-project 22d7058, copied into plain repos:

  • llvm: llvm/{include,lib,unittests,tools,utils/TableGen} + third-party/unittest, 9,373 files;
  • clang: clang/{include,lib,unittests,tools} as the root, so the path starts with unittests/;
  • lldb: lldb/{include,source,unittests,tools};
  • offload: the whole subproject;
  • clangd: the whole subproject.
reposource files newly test / newly suiteedges base → fixremovedaddedretargeted
llvm95 / +7281,037,961 → 1,035,3272,758, all wrong124: 106 right, 18 wrong1 wrong → wrong
clang71 / +286678,279 → 678,154138, all wrong13, all wrong—
lldb79 / +244213,733 → 213,65088: 28 wrong, 60 SWIG stand-ins5, all wrong—
offload95 / +116,061 → 15,980103, all wrong22: 1 right, 21 wrong1 wrong → right
clangd22 / +10335,393 → 34,833560, all wrong——
glog8identical———
breakpad2identical———
leveldb, prometheus, cpython0identical———

The two earlier mains (ed199e60 + #2434, then b635dd46) gave the same categories in each step; LLVM's suite step removed 2,682 and 2,762 edges there.

Removed (3,647)

  • 3,587 wrong: production code linked to a test's own declarations. In LLVM, Ctx.TTI in VPlanRecipes.cpp linked to LLVMContext Ctx; in RandomIRBuilderTest.cpp (578 Ctx sites), KV.second to BLAKE3Test.cpp's using KV = std::pair<…> (452), OpInfo.ConstraintVT to X86MCDisassemblerTest.cpp's struct OpInfo (332), V.second to a YAMLIOTest.cpp struct V (397). Also clangd's S to TraceTests.cpp (248) and its D to a test-model TestEnum::D (147), offload's Device->Device to a conformance-test struct Device, and lldb's getBuffer and status to FileSystemTest.cpp's DummyFile.
    • All sources are production folders (lib/, include/, tools/, source/, …).
    • The exception is 10 sites in the vendored googletest, which are also wrong: gmock's own StringType template parameter linked to SmallStringTest.cpp's alias.
    • A random sample of 40 LLVM sites, read line by line, was 40/40 wrong.
    • The isTestPath half also stops production method guesses from landing on a unittests helper: Pair.getKey() → FoldingSet.cpp's KeyedPair::getKey, and clangd's OutputBuffer.clear() → ConfigTesting.h's CapturedDiags::clear.
  • 60 lldb stand-ins. Production calls like SWIGBridge::LLDBSwigPythonBreakpointCallbackFunction, python::LLDBSWIGPython_CastPyObjectToSBValue and luaopen_lldb had linked to the stubs in unittests/ScriptInterpreter/{Python/PythonTestSuite.cpp,Lua/LuaTests.cpp}. Those are the only bodies in the repository: the real ones are generated by SWIG at build time, and the unit tests link the stubs in their place. The class body in SWIGLuaBridge.h is not a node (C++ in-class declarations without a body aren't extracted), so these calls now link nothing. That's the same as for a stub in a tests/ folder today, and better than pointing an agent at a return false; stub as the code production runs.

Added (164)

  • 107 right.
    • 99 production calls a unittests fixture's namesake had kept ambiguous now resolve. 92 are TLI.getTypeToTransformTo(…) in LegalizeFloatTypes.cpp and friends → TargetLoweringBase::getTypeToTransformTo; SelectionDAGTestBase.h had a method of that name. The other 7 are KeyInfo::getLatticeKeyFromValue in SparsePropagation.h → the only production LatticeKeyInfo specialization, in CalledValuePropagation.cpp.
    • Newly-test helpers reach test support: 4 ::testing::Values → googletest, and 3 TmpFile.path() → llvm::unittest::TempFile::path.
    • offload's TestRunner.hpp TestType::create → GpuMathTest::create, and its ExpectedTest->run moves from RPCServerTy::ServerThread::run to GpuMathTest::run. TestType defaults to GpuMathTest<Func>.
  • 57 wrong, from two weaknesses main already has:
    • 52: a C/C++ test reference sees every other test file. A newly-test helper now does too: offload's ol_init_args_t Args local → a C test program's struct Args (21), LLVM's fs::status and fs::openFileForRead → VirtualFileSystemTest.cpp's InterceptorFS (17), clang's Visitor → another test's class.
    • 5: in clang, removing a unittests rival let a different wrong guess win. WorkList.top() → interp::InterpStack::top. The same mechanism produced LLVM's 92 right getTypeToTransformTo links.
  • Net: −3,587 wrong, +57 wrong, +107 right, −60 stand-ins.

Controls

  • leveldb, prometheus and cpython are byte-identical (natural-key dump hashes), and no file in them is reclassified.
  • CPython's Lib/unittest/ keeps all 37,820 edges into it from outside, including the 13 from Lib/doctest.py.

Query time

glog's and Breakpad's graphs are identical, but their unit-test folders now count as tests:

  • glog's codegraph affected src/glog/logging.h goes from 12 to 17 tests: the dcheck_unittest/, includes_unittest/, log_severity_unittest/ and package_config_unittest/ programs.
  • Breakpad's affected on minidump_format.h now lists unittests/dump_analysis.h.
  • Search ranking and the dead-code report treat these folders as tests too.

Tests

  • __tests__/is-test-file.test.ts: unittests/ and foo_unittest/ paths are tests. A bare unittest/ is not (Lib/unittest/*, third-party/unittest/googletest/…, unittest2/).
  • __tests__/unittests-directories.test.ts (new):
    • production Ctx / Severity locals no longer link to a unittests/ or foo_unittest/ declaration (the suite rule);
    • TLI.getTypeToTransformTo resolves despite SelectionDAGTestBase.h's namesake (the isTestPath rule).
  • __tests__/cli-affected-test-conventions.test.ts: codegraph affected lists unittests/ADT/FoldingSet.cpp.
  • Red on main (4 tests failed). With the isTestPath rule alone, the suite test still fails and the fixture-method test passes, so each rule has its own pin.

Full suite on Windows (main 1ce1167e + this change): 6,897 passed, 0 failed, 82 skipped.

🤖 Generated with Claude Code