fix(c,cpp): a file named like a Google unittest, foo_unittest.cc, is a test

Merged
CO
9d ago
+193-8

Summary

Google names its C++ tests foo_unittest.cc, not foo_test.cc. That covers protocolbuffers/protobuf (62 files), google/breakpad (77), google/glog (12) and Chromium; Chromium's Python tests are foo_unittest.py. isTestPath's separator-delimited pattern /[._-](test|tests|spec|specs)\.[a-z0-9]+$/ needs a separator right before test, and in unittest that position holds a t. So every one of these files read as production code:

  • search and codegraph_explore ranked it alongside the code it tests;
  • codegraph affected never listed it (glog: a change to src/demangle.cc reported no tests at all);
  • dead code listed its fixtures and helpers;
  • the resolver's test rules ran backwards for it:
    • production code resolved into a unittest's own declarations (Breakpad's stack_frame_entries_.size() onto a unittest's StackHelper::size, a template's AddressType() onto a unittest's typedef);
    • a unittest's own calls into test-suite code were treated as production calls.

Change

  • isTestPath (src/search/query-utils.ts): unittest and unittests join the separator-delimited suffixes, so foo_unittest.cc, foo-unittest.cpp, foo.unittest.js and run_all_unittests.cc all count. A bare unittest.go still does not. prometheus's cmd/promtool/unittest.go is the production code behind promtool test rules, called from main.go, so no unittest. prefix rule was added, and no unittest/ directory rule either: CPython's Lib/unittest/ is the framework itself.
  • isTestSuitePath (src/resolution/name-matcher.ts): yes, these files count as test suites, like foo_test.cc. I decided this from a middle arm (isTestPath only) indexed next to the full fix on the pre-#2421 main. The suite rule alone removed 130 edges (protobuf 84, Breakpad 46), and every one was a wrong production → unittest edge. It removed nothing right.

Validation

Arms: main ed199e60 vs ed199e60 + this change. Both use the same freshly built kernel and an identical compiled engine apart from query-utils.js and name-matcher.js; the shipped dist/ was checked byte-for-byte against the fix arm. Each repo was fully indexed per arm, and sites were keyed by (kind, source node, line, col, refName).

Controls, byte-identical dumps: leveldb and rocksdb (C++ with _test.cc), prometheus (Go, including the production unittest.go), django (Python), excalidraw (TS), okhttp (Kotlin), tokio (Rust). glog's graph is byte-identical too: its unittests reach nothing new, and only the query-time behaviour below changes.

reporemovedaddedretargetedmetadata only
protocolbuffers/protobuf8418212
google/breakpad555600

Every removed and retargeted site was read against the source.

protobuf

  • 84 removed, all wrong. Each is production code that had resolved into a unittest:
    • 28× GetField<T>(message, field) in generated_message_reflection.cc → descriptor_unittest::HasHasbitTest::GetField;
    • 20× a callable parameter f(…) (map.h, json lexer/writer, descriptor.h, field_mask_util.cc) → a test-local TEST_F::X::f;
    • 15× ParseFrom<kParse>(…) in message_lite.cc → a free ParseFrom in lite_unittest.cc;
    • 15× absl::Cord(…) → a misparsed node in lite_unittest.cc;
    • Win32 ReadFile ×2 → CommandLineInterfaceTest::ReadFile;
    • BackUp ×2 inside ZeroCopy*Stream → a test-local struct;
    • template parameter F → a unittest function;
    • IsLazyField → a unittest helper.
  • 18 added, all wrong. Once a unittest is a test, it sees every test suite, as every _test.cc does today:
    • 12× error_collector.last_error() in descriptor_unittest.cc → compiler::java::SimpleErrorCollector (name_resolver_test.cc). The right class is in descriptor_test_utils.h, which the file includes, but resolveMethodOnType takes the first indexed of five same-named classes.
    • 6× bare TestSourceDir() in the csharp/ruby generator unittests → TestUtil::TestSourceDir (test_util2.h, not included) instead of googletest.h's google::protobuf::TestSourceDir.
    • This is pre-existing behaviour for test files. On main, protobuf's _test.cc files already have 48 edges into test files outside their translation unit, and rocksdb's have 23,003 (e.g. Flush() in db_compaction_test.cc, a DBTestBase, lands on column_family_test.cc's fixture 1,174 times; I checked that one, and it is wrong). I filed it as a follow-up rather than widening this PR.
  • 2 retargeted, wrong → wrong (receiver-word guesses, @0.65): input->Next in importer_unittest.cc and stream_->BackUp in zero_copy_sink.h. The latter no longer points into a unittest.
  • 12 metadata only. The targets stay the same and right: parser_->GetSyntaxIdentifier() and TestGenerator::GetResolvedSourceFeatureExtension. Confidence moves 0.7 → 0.65 because a unittest's call now also counts test methods as candidates.

Breakpad

  • 55 removed.
    • 53 wrong, now unresolved:
      • 22× container size() (std::vector/std::string members, mapping.data.size(), …) → a unittest's StackHelper::size;
      • 14× endianness() in cfi_assembler.h / synth_elf.cc → another unittest's WithConfiguration;
      • 4× Range, 4× template ValueType, 4× template AddressType, DumpSymbols → unittest-local types;
      • Win32 ReadFile ×2;
      • stabs.endianness() and segment2.Size().
    • 2 right edges lost: strings_found.Size() on ByteBuffer symbols_found, strings_found; (macho_reader_unittest.cc:1851, 1888). C++ receiver inference never reads the second declarator's type, so these were lucky guesses: the only production Size once test methods were filtered out. Filed as a follow-up.
  • 56 added, all right. A unittest's calls on test_assembler::Section/Label receivers, e.g. stack_section.GetContents, frame2_sp.Value and fat.start. I checked the receivers' declarations: Section stack_section;, Label frame1_sp, frame2_sp, frame1_rbp;, test_assembler::Section fat;.

Re-checked on the final base after two merge rounds (#2430, #2442, #2449, then #2433, #2444), on main 3de5bef0 vs the squash 1fb8691d. #2449 changes C++ extraction. The changed-site sets on protobuf (116) and Breakpad (111) are identical to the ed199e60 measurements above, compared site by site with the before/after targets, and glog is still byte-identical.

Query-time effects, same arms:

  • codegraph affected src/demangle.cc src/demangle.h on glog: 0 → 12 tests, all *_unittest.cc.
  • codegraph affected wire_format.cc wire_format.h on protobuf: 134 → 190 tests (+56 *_unittest.cc).
  • Dead code with exported symbols included: Breakpad 444 → 435, glog 34 → 28. The difference is exactly the unittest-local fixtures that were listed.

Overlap with #2421 (merged)

#2421 makes a test-named C/C++ file visible to the translation units that include it, and this PR fills the gap noted there: isTestPath didn't know *_unittest.cc. On the pre-#2421 main (e0bbb662), this change alone added 587 protobuf edges and 863 Breakpad edges, keyed by source line/col, refName and target:

  • 563 and 807 of them, the unittests' calls into the test helpers they include, are now on main through #2421;
  • 4 (wrong data() guesses on std::string / absl::string_view) are gone through #2413;
  • the remaining 20 and 56 are the added and retargeted rows above.

The two compose without conflict: a _unittest.h is now a suite, and only unittests include one.

Tests

  • __tests__/unittest-named-files.test.ts (new). A Python device_unittest.py reaches FakeDevice.reboot in tests/; production stack_frame_entries_.size() and AddressType() don't resolve into a unittest. A C++ TestUtil::SetAllFields guard rides along.
  • __tests__/is-test-file.test.ts: positive forms, plus cmd/promtool/unittest.go and Lib/unittest/case.py as negatives.
  • __tests__/cli-affected-test-conventions.test.ts: codegraph affected reports a C++ _unittest.cc.
  • Red/green on the merged branch: the 4 new tests fail on main's source with the expected assertions and pass with the change. tsc is clean on src and on the three test files.
  • Full suite on Windows (before merging main): 110 failures at 100% CPU with 100+ node processes from other sessions, all timeouts. A serial rerun with 120 s timeouts left 4 known load-sensitive ones (function-ref #1820's own 60 s limit, mcp-staleness-banner, mcp-status-freshness, mcp-writer-lock), none touching test-file naming. After merging main (ed199e60), a serial run of the 31 test files closest to the change passed 379/379. Those were the three above, prod-to-test-suite, kotlin-top-level-visibility, ui-entrypoints-api, dead-code, all 12 cpp-* suites and the 15 files main's merge touched. The 4 load-sensitive tests fail the same way on main's source on this machine.

Not changed

Two private copies of the pattern keep their own lists:

  • the viewer's search ranking nudge in src/ui-server/api/search.ts (viewer unreleased);
  • the explore seed filter in src/mcp/tools.ts, which doesn't know _test.go either.

Follow-ups (filed as tasks)

  • C/C++ test files resolving into test files outside their translation unit (the 18 above; rocksdb's 23k).
  • T a, b;: read every declarator's type.
  • unittests/ and foo_unittest/ directories (Breakpad's src/client/windows/unittests/, glog's src/dcheck_unittest/, LLVM's unittests/).

🤖 Generated with Claude Code