Skip to content

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

Merged
colbymchenry merged 5 commits into
mainfrom
claude/nice-einstein-5d0ffa
Oct 7, 2026
Merged

colbymchenry merged 5 commits into
mainfrom
claude/nice-einstein-5d0ffa

Conversation

@colbymchenry

@colbymchenry colbymchenry commented Oct 7, 2026 •

Copy link
Copy Markdown
Owner

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-fix(c,cpp): a file named like a test is visible to the code that includes it #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.

repo removed added retargeted metadata only
protocolbuffers/protobuf 84 18 2 12
google/breakpad 55 56 0 0

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:

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 Caller/impact graph misses methods passed as first-class references (callbacks), e.g. executor.submit(obj.method, …) #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

colbymchenry and others added 5 commits October 7, 2026 05:32
…a test

Google names C++ tests `foo_unittest.cc` (protobuf, Breakpad, glog,
Chromium), not `foo_test.cc`. The test-file check wanted a separator right
before `test`, so each one read as production code: search and explore
ranked it alongside the code it tests, `codegraph affected` never listed
it, and the resolver's test rules ran backwards for it. A unittest could
not reach the test-suite helpers it calls (`TestUtil::SetAllFields` in
test_util.h), and the receiver guess and the test-suite rule let
production calls land on a unittest's own declarations
(`handler_stack_->size()` on a unittest's `StackHelper`, a template's
`AddressType()` on a unittest's typedef).

`unittest` and `unittests` join the separator-delimited test suffixes
(`foo_unittest.cc`, `foo-unittest.cpp`, `foo.unittest.js`), and a file
named so is a test suite to the resolver, as `foo_test.cc` is. A bare
`unittest.go` stays production code: promtool's runs rule tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#2421 links a C/C++ unittest to the test helpers it includes, and #2413
keeps a `std::vector` receiver off project methods, so two of the
regression test's cases passed on main without this change. Pin what only
this change does: a Python `*_unittest.py` reaching a `tests/` fake, and
production `stack_frame_entries_.size()` (`vector` through `using
std::vector`) and `AddressType()` staying out of a unittest. The test
covers more than C++ now, so it is renamed. The CHANGELOG entry no longer
claims what #2421's entry already says, and the indexing setups get the
60 s limit the sibling C++ tests use.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant