From 1d3619d65728931bfb1dd0b923ac8aabfa897674 Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Sat, 10 Oct 2026 18:09:06 +0000 Subject: [PATCH] fix(c,cpp): a file in a unittests/ or foo_unittest/ folder is a test (#2484) isTestPath knew `tests/`, `test/`, `__tests__/`, CamelCase source sets and `foo-test/` modules, but not the folders LLVM, Clang, clangd, lldb, offload and Breakpad keep their unit tests in (`unittests/`), nor glog's `dcheck_unittest/`-style test programs: `/unittests/` holds `tests/` only after a `t`, and the `foo_test/` form wants a separator right before `test`. So a helper there with no test-like name (LLVM's CountCopyAndMove.h, SelectionDAGTestBase.h, Breakpad's dump_analysis.cc) read as production code. isTestPath now also counts a `unittests/` folder and a `foo_unittest/` or `foo-unittests/` one. A bare `unittest/` stays production: CPython's `Lib/unittest/` is the framework, and LLVM vendors googletest under `third-party/unittest/`. isTestSuitePath counts the same folders as suites, as it counts `tests/`: their files build into test binaries and never link into the program, and glog builds each `foo_unittest/` folder as a test program of its own (a `foo-test/` module stays out, as shipped test support). That matters for C/C++ because the suite rule's CamelCase file names skip `.cpp`, so on main none of LLVM's `*Test.cpp` files were suites and production locals named `Ctx`, `KV`, `V` or `OpInfo` linked to what a unit test declares. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + .../cli-affected-test-conventions.test.ts | 7 + __tests__/is-test-file.test.ts | 31 ++++ __tests__/unittests-directories.test.ts | 153 ++++++++++++++++++ src/resolution/name-matcher.ts | 13 +- src/search/query-utils.ts | 6 +- 6 files changed, 204 insertions(+), 7 deletions(-) create mode 100644 __tests__/unittests-directories.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a3a3cde4..c14bfcb90 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -76,6 +76,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - `codegraph sync` and the file watcher now link a Shopify section or snippet to the files that name it when it is added after them, or comes back after being deleted, whether they name it in a `{% render %}`, `{% include %}` or `{% section %}` tag or as a section `type` in a JSON template. Before, the link appeared only once a file naming it changed or the project was re-indexed, even in a repository with a single theme, so the new section or snippet looked unused and `codegraph affected` missed the files that use it. Re-index Shopify themes after upgrading to pick up links an earlier version missed. - `codegraph sync` and the file watcher now link a route to the page or layout it loads lazily when that file is added after the router was indexed, comes back after being deleted, or gets the component the route renders in a later edit, whether or not the import names the file's extension: React Router's `lazy: () => import('./pages/Team')`, Vue Router's `component: () => import('@/views/Login')` and Angular's `loadComponent: () => import('./home/home.component')`. Before, the route stayed unlinked until the router file changed or the project was re-indexed, so the page looked unused and `codegraph affected` missed the tests that reach it through the router. Re-index after upgrading to pick up links an earlier version missed. - A file named the way Google names its tests, like `wire_format_unittest.cc` in protocolbuffers/protobuf, `logging_unittest.cc` in google/glog or a Python `foo_unittest.py`, now counts as a test, as `wire_format_test.cc` already did. Before, it was taken for production code: search and `codegraph_explore` ranked it alongside the code it tests, `codegraph affected` never listed it, and calls in production code could link to a class or type a unittest declares for itself, as google/breakpad's `stack_frame_entries_.size()` did to a unittest's `StackHelper`. Re-index projects that have such files after upgrading. +- A file in a `unittests/` folder, where llvm/llvm-project and google/breakpad keep their unit tests, or in a folder named like a unittest, such as `dcheck_unittest/` in google/glog, now counts as a test whatever its own name says, like LLVM's `CountCopyAndMove.h` helper. Before, such a file was taken for production code unless its name said test, and none of them were kept apart from the program the way a `tests/` folder is: search and `codegraph_explore` ranked them alongside production code, `codegraph affected` missed them, and production code linked to a unit test's own declarations, as LLVM's `Ctx` and `OpInfo` variables did to ones its unit tests declare. A folder named just `unittest/`, like CPython's `Lib/unittest/`, is still production code. Re-index projects that have such folders after upgrading. - `codegraph sync` and the file watcher now link a file to a module it imports under a name of its own when the module is added after the file was indexed, comes back after being deleted, or gets the export in a later edit: a default import like `import tagsController from './tag/tag.controller'`, a namespace import like `import * as api from './api'`, or an aliased one like `import { Component as GroupsPageWrapper } from './groups_page'`, in JavaScript, TypeScript, Vue, Svelte and Astro. Before, the import and every use of the name, such as a call, `new`, a base class, a JSX or template tag or an Express `app.use` mount, stayed unlinked until the importing file changed or the project was re-indexed, so the module looked unused and `codegraph affected` missed the files that use it. Re-index after upgrading to pick up links an earlier version missed. - In C, C++ and Objective-C, a struct or class member declared as a pointer, reference, array or function, like `SharedState* shared;`, `jv elements[];` or `virtual Status Put(…) = 0;`, no longer makes the struct look like it inherits from the member's type. Before, `codegraph_explore` and impact listed those types as base classes, every struct holding a pointer to a type as one of its subclasses, and a method of that struct as an override of the type's method with the same name. Real base classes like `class Derived : public Base`, Objective-C superclasses and Go's embedded fields are unchanged. Re-index C, C++ and Objective-C projects after upgrading. - In C++, a call on a variable, parameter or member declared through a `typedef` or `using` alias now links to the method of the type the alias names, with the alias looked up the way C++ does it: in the calling function, its class and the classes that class inherits from, then the namespaces around it. In google/leveldb, `table_.Insert(buf)` on a `Table table_;` declared next to `typedef SkipList Table;` now reaches `SkipList::Insert` instead of the unrelated `HandleTable::Insert`, and `Table::Iterator iter(&table_);` reaches `SkipList::Iterator`'s constructor. A class that only shares the alias's name is no longer taken for it, and when the aliased type has no such method, as with an alias of `std::vector` or of a template parameter, no method is guessed from the receiver's name. Re-index C++ projects after upgrading. diff --git a/__tests__/cli-affected-test-conventions.test.ts b/__tests__/cli-affected-test-conventions.test.ts index 7418fde25..f5f314263 100644 --- a/__tests__/cli-affected-test-conventions.test.ts +++ b/__tests__/cli-affected-test-conventions.test.ts @@ -44,6 +44,9 @@ describe('codegraph affected — test-file conventions (#1507)', () => { w('src/wire_format.h', '#pragma once\n\nint ByteSize(int value);\n'); w('src/wire_format.cc', '#include "wire_format.h"\n\nint ByteSize(int value) { return value; }\n'); w('src/wire_format_unittest.cc', '#include "wire_format.h"\n\nvoid ComputesByteSize() { ByteSize(1); }\n'); + w('include/llvm/ADT/FoldingSet.h', '#pragma once\n\nint FoldingSetSize();\n'); + w('lib/Support/FoldingSet.cpp', '#include "llvm/ADT/FoldingSet.h"\n\nint FoldingSetSize() { return 0; }\n'); + w('unittests/ADT/FoldingSet.cpp', '#include "llvm/ADT/FoldingSet.h"\n\nvoid SizesTheSet() { FoldingSetSize(); }\n'); const cg = CodeGraph.initSync(dir); await cg.indexAll(); cg.close(); @@ -66,6 +69,10 @@ describe('codegraph affected — test-file conventions (#1507)', () => { expect(affected(dir, ['src/wire_format.h', 'src/wire_format.cc'])).toEqual(['src/wire_format_unittest.cc']); }); + it("reports a file in LLVM's unittests/ tree, whatever its name", () => { + expect(affected(dir, ['include/llvm/ADT/FoldingSet.h', 'lib/Support/FoldingSet.cpp'])).toEqual(['unittests/ADT/FoldingSet.cpp']); + }); + it('still honours an explicit --filter glob', () => { expect(affected(dir, ['math.go', '--filter', '*_test.go'])).toEqual(['math_test.go']); expect(affected(dir, ['math.go', '--filter', '*.spec.ts'])).toEqual([]); diff --git a/__tests__/is-test-file.test.ts b/__tests__/is-test-file.test.ts index 08b67e7d4..282bcf27c 100644 --- a/__tests__/is-test-file.test.ts +++ b/__tests__/is-test-file.test.ts @@ -80,6 +80,37 @@ describe('isTestFile', () => { expect(isTestFile('cmd/promtool/unittest.go')).toBe(false); }); + it('flags everything in a unittests/ or foo_unittest/ directory', () => { + // LLVM's and Breakpad's unit-test trees hold helpers with no test-like + // name, and glog builds each `*_unittest/` directory as a test program. + for (const file of [ + 'llvm/unittests/ADT/CountCopyAndMove.h', + 'unittests/ADT/FoldingSet.cpp', + 'clang-tools-extra/clangd/unittests/Annotations.cpp', + 'src/client/windows/unittests/dump_analysis.cc', + 'src/dcheck_unittest/glog_dcheck.cc', + 'src/includes_unittest/glog_includes_logging.cc', + 'tools/net-unittests/fake_socket.py', + ]) { + expect(isTestPath(file), file).toBe(true); + expect(isTestFile(file), file).toBe(true); + } + }); + + it('does NOT flag a bare unittest/ directory', () => { + // CPython's `Lib/unittest/` is the framework, and so is the googletest LLVM + // vendors under `third-party/unittest/`; `unittest2` is the framework's backport. + for (const file of [ + 'Lib/unittest/mock.py', + 'Lib/unittest/__init__.py', + 'third-party/unittest/googletest/src/gtest.cc', + 'unittest2/case.py', + ]) { + expect(isTestPath(file), file).toBe(false); + expect(isTestFile(file), file).toBe(false); + } + }); + it('does NOT flag production files that merely contain "test" lowercase', () => { // The fix is capital-led so camelCase boundaries distinguish these. expect(isTestFile('src/latest/loader.kt')).toBe(false); diff --git a/__tests__/unittests-directories.test.ts b/__tests__/unittests-directories.test.ts new file mode 100644 index 000000000..7664c75ee --- /dev/null +++ b/__tests__/unittests-directories.test.ts @@ -0,0 +1,153 @@ +/** + * LLVM keeps its unit tests in `unittests/` (`llvm/unittests/`, + * `clang/unittests/`, clangd's `unittests/`), Breakpad in + * `src/client/windows/unittests/`, and glog builds each `foo_unittest/` + * directory as a test program of its own. The test-file check knew `tests/` + * but not these, so a helper there with no test-like name read as production + * code, and none of the tree counted as a test suite: production locals named + * `Ctx` or `OpInfo` linked to a test's own `LLVMContext Ctx;`, and a test + * fixture's method kept a production call from resolving at all. + */ +import { describe, it, expect, afterAll, beforeAll } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; + +let root = ''; +let cg: CodeGraph; + +beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-unittests-dirs-')); + const files: Record = { + // A call the receiver's type doesn't settle, so its method is guessed by + // name — and LLVM's SelectionDAGTestBase.h, a test fixture, has a method of + // that name too. + 'include/llvm/CodeGen/TargetLowering.h': `#pragma once + +namespace llvm { + +class TargetLoweringBase { + public: + int getTypeToTransformTo(int VT) const { return VT * 2; } +}; + +class TargetLowering : public TargetLoweringBase {}; + +} // namespace llvm +`, + 'lib/CodeGen/SelectionDAG/LegalizeTypes.h': `#pragma once + +#include "llvm/CodeGen/TargetLowering.h" + +namespace llvm { + +class DAGTypeLegalizer { + const TargetLowering &TLI; + + public: + explicit DAGTypeLegalizer(const TargetLowering &T) : TLI(T) {} + int softenFloat(int VT); +}; + +} // namespace llvm +`, + 'lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp': `#include "LegalizeTypes.h" + +namespace llvm { + +int DAGTypeLegalizer::softenFloat(int VT) { + return TLI.getTypeToTransformTo(VT); +} + +} // namespace llvm +`, + 'unittests/CodeGen/SelectionDAGTestBase.h': `#pragma once + +#include "llvm/CodeGen/TargetLowering.h" + +namespace llvm { + +class SelectionDAGTestBase { + protected: + const TargetLowering *Lowering = nullptr; + int getTypeToTransformTo(int VT) { return Lowering->getTypeToTransformTo(VT); } +}; + +} // namespace llvm +`, + // A production parameter named like a variable a unittest declares. + 'lib/Object/WasmObjectFile.cpp': `namespace llvm { + +struct ReadContext { + const unsigned char *Ptr; + const unsigned char *End; +}; + +unsigned readByte(ReadContext &Ctx) { + if (Ctx.Ptr == Ctx.End) return 0; + return *Ctx.Ptr++; +} + +} // namespace llvm +`, + 'unittests/FuzzMutate/RandomIRBuilderTest.cpp': `namespace { + +struct LLVMContext { + int Kind = 0; +}; + +LLVMContext Ctx; + +} // namespace + +int RandomIRBuilderTestKind() { return Ctx.Kind; } +`, + // The same through one of glog's test programs. + 'src/logging.cc': `struct LogSeverityNames { + const char *Name; +}; + +const char *SeverityName(const LogSeverityNames &Severity) { + return Severity.Name; +} +`, + 'src/log_severity_unittest/glog_log_severity_constants.cc': `struct Severity { + int Value; +}; + +static Severity Fatal{3}; + +int main() { return Fatal.Value == 3 ? 0 : 1; } +`, + }; + for (const [rel, content] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); + fs.writeFileSync(path.join(root, rel), content); + } + cg = await CodeGraph.init(root, { index: true }); +}, 60_000); + +afterAll(() => { + cg?.close(); + if (root) fs.rmSync(root, { recursive: true, force: true }); +}); + +const edgesFrom = (file: string) => { + const ids = cg.getNodesInFile(file).map((n) => n.id); + return cg.getOutgoingEdgesFrom(ids).filter((e) => e.kind !== 'contains').map((e) => cg.getNode(e.target)!); +}; + +describe('unittests/ and foo_unittest/ directories are test suites', () => { + it("production code never resolves into a unit test's own declarations", () => { + for (const file of ['lib/Object/WasmObjectFile.cpp', 'src/logging.cc']) { + const intoTests = edgesFrom(file).map((n) => n.filePath).filter((f) => /unittests?\//.test(f)); + expect(intoTests, file).toEqual([]); + } + }); + + it("a test fixture's method no longer keeps a production call from resolving", () => { + expect(edgesFrom('lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp').map((n) => n.qualifiedName)) + .toContain('llvm::TargetLoweringBase::getTypeToTransformTo'); + }); +}); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 079139e45..7714cc34e 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -1553,11 +1553,12 @@ const JVM_CALLABLE_KINDS: ReadonlySet = new Set(['method', 'function']); const JVM_TYPE_KINDS: ReadonlySet = new Set(['class', 'interface', 'enum', 'struct', 'trait', 'type_alias', 'annotation']); /** - * A test suite — a test source set, a `tests/` / `__tests__/` / `spec/` - * directory, a `FooTest.kt` / `test_foo.py` / `foo.test.ts` / `foo_unittest.cc` - * file — as opposed to test-support code a project ships (`testing/`, - * `fakes/`, a `*-test` module like kotlinx-coroutines-test), which its own - * code may use. + * A test suite — a test source set, a `tests/` / `__tests__/` / `spec/` / + * `unittests/` directory, a `FooTest.kt` / `test_foo.py` / `foo.test.ts` / + * `foo_unittest.cc` file — as opposed to test-support code a project ships + * (`testing/`, `fakes/`, a `*-test` module like kotlinx-coroutines-test), which + * its own code may use. A `foo_unittest/` directory is a suite, as a + * `foo_unittest.cc` file is: glog builds each one as a test program of its own. */ function isTestSuitePath(filePath: string): boolean { if (!isTestPath(filePath)) return false; @@ -1568,7 +1569,7 @@ function isTestSuitePath(filePath: string): boolean { if (name.startsWith('test_') || /[._-](?:test|tests|unittest|unittests)\.[a-z0-9]+$|[._](?:spec|specs)\.[a-z0-9]+$/.test(name) || // CamelCase suffixes where the language names tests so: not `useTests.ts`, a React hook. /(?:Test|Tests|TestCase)\.(?:java|kt|kts|swift|cs|scala|groovy|m|mm|vb|fs)$/.test(original) || name === 'conftest.py') return true; - return /(?:^|\/)(?:tests?|__tests__|specs?|e2e)\//.test(lower) || /(?:^|\/)[A-Za-z0-9]*(?:Test|Tests|Spec)\//.test(filePath); + return /(?:^|\/)(?:tests?|__tests__|specs?|e2e|unittests|[\w.]+[-_]unittests?)\//.test(lower) || /(?:^|\/)[A-Za-z0-9]*(?:Test|Tests|Spec)\//.test(filePath); } const fileStem = (filePath: string): string => { diff --git a/src/search/query-utils.ts b/src/search/query-utils.ts index e51cbe5d4..91bbdd09e 100644 --- a/src/search/query-utils.ts +++ b/src/search/query-utils.ts @@ -345,7 +345,11 @@ export function isTestPath(filePath: string): boolean { // Test-support modules and doubles by directory name: Gradle's // `core/data-test/`, `core/datastore-test/`, `:testing`; Go's `testdata/`; // `testutil(s)/`, `test-utils/`, `fakes/`, `mocks/`, `__mocks__/`. - /(?:^|\/)(?:[\w.]+[-_]test(?:s|ing)?|testdata|testutils?|test[-_]utils?|fakes?|mocks?|__mocks__|stubs)\//.test(lower) + /(?:^|\/)(?:[\w.]+[-_]test(?:s|ing)?|testdata|testutils?|test[-_]utils?|fakes?|mocks?|__mocks__|stubs)\//.test(lower) || + // Unit-test trees: LLVM's and Breakpad's `unittests/`, glog's + // `dcheck_unittest/`. Not a bare `unittest/`: CPython's `Lib/unittest/` is + // the framework itself. + /(?:^|\/)(?:unittests|[\w.]+[-_]unittests?)\//.test(lower) ) { return true; }