Conversation
The suite wires a full production symbol-knowledge stack through BuildsKnowledgeStackTrait and asserts on fixtures resolved end-to-end through SymbolResolver, ExpressionResolver and NativeTypeSource. That is an integration test wearing a unit-test filename. Rename it to say what it does; a real unit test for the handler follows in the next commit. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The handler already takes DocumentSourceInterface and CodeResolverInterface, so a real unit test can be written by stubbing them directly. Covers the handler's own behavior: method claim, short-circuits on malformed params and unopened documents, null passthrough when the cursor is outside a call, and the presenter+parameter shape it emits when a CallContext is returned. The resolver's ability to identify the call at a given cursor is exercised by the sibling integration test. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #657 +/- ##
============================================
+ Coverage 99.65% 99.69% +0.04%
Complexity 1903 1903
============================================
Files 136 136
Lines 4882 4882
============================================
+ Hits 4865 4867 +2
+ Misses 17 15 -2 ☔ View full report in Codecov by Harness. |
Integration tests already have their own directory. Adjust the namespace and pull OpensDocumentsTrait in through its full name now that it is no longer a sibling. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two disjoint suites: unit (everything outside tests/Integration) and integration (tests/Integration only). Running phpunit without a --testsuite still covers both, so composer test is unchanged. A follow-up composer script can select the unit suite for a fast inner-loop run. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Every textDocument/position handler unit test needs the same two request builders. Extract them once so the next handler test does not re-derive the shape. The trait is method-agnostic; each test passes its own method name and URI, so a bad params shape or an unopened document is expressible in one line without a per-file adapter. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The suite builds a full production KnowledgeStack over inline documents and asserts on definition locations resolved end-to-end. Its CoversClass list already names ExpressionResolver and ResolvedTypeOnly. Move it to tests/Integration and rename to match. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Same shape as the SignatureHelpHandler unit test: stub the two interface collaborators and cover the handler's own behavior. The resolver's ability to identify what a cursor points at is the sibling integration tests job. The two new branches over signature help are the two paths through getDefinitionLocation(): a resolved symbol without one yields null, and one with a Location returns its toLspLocation() shape. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Firehed
changed the title
Owner
Author
|
Superseded by #660 (phase 1: renames) and its stacked phase 2 (unit tests) — this PR mixed both concerns in one branch. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces the fixture-driven "unit" test files for two handlers with true unit tests that stub the two interfaces each handler declares. The suites that build a full production
KnowledgeStackin setUp are renamed and moved totests/Integration/, so nothing is lost — they're just called what they are.Handlers already take interfaces (
DocumentSourceInterface,CodeResolverInterface), so this is test-side only: no changes undersrc/.What changed
SignatureHelpHandlerTestandDefinitionHandlerTestundertests/Handler/are now true unit tests. They stub their two interface collaborators and assert on the handler's own behavior: method claim, short-circuits on malformed params and unopened documents, and the exact wire shape emitted for a resolved result.SignatureHelpHandlerIntegrationTest/DefinitionHandlerIntegrationTestand moved totests/Integration/. Their#[CoversClass]lists (ExpressionResolver, ResolvedTypeOnly) already said what they are.BuildsHandlerRequestsTraitshared by both unit tests; each passes its own method name and URI to method-agnostic request builders.phpunit.xml.distnow defines two disjoint testsuites,unitandintegration.phpunitwithout a--testsuiteruns both, so CI andcomposer testare unchanged.Why
Every handler already depends on interfaces, so its unit test can be a real unit test with no production change. The existing suites carry the pattern that predates those interfaces and each rebuilds
KnowledgeStack::forProject's output in setUp — the duplicate wiring route the design rules call out. Shrinking the surface that reaches forBuildsKnowledgeStackTraitis the prerequisite for later movingServer::forProject/KnowledgeStack::forProjectinto container factories.What follows
HoverHandlerTestandCompletionHandlerTest.forProjectmethods can be deleted.🤖 Generated with Claude Code