test(frontend): render the agent panel with its real children - #7629
Open
aglinxinyuan wants to merge 2 commits into
Open
test(frontend): render the agent panel with its real children#7629aglinxinyuan wants to merge 2 commits into
aglinxinyuan wants to merge 2 commits into
Conversation
agent-panel.component.html reported 0 of 49 lines covered while its own .ts sat at 133/133 with nothing missed. That pairing can only come from attribution loss, not from undertesting - this template is one of the better-tested in the frontend. The cause is apache#7458: the spec stubs its children out through TestBed.overrideComponent, and any override re-JITs the component from its decorator metadata, leaving the re-compiled template with no source map back to the .html. Adds a describe block that renders the component with its real children, taking the template from 0/49 to 49/49 lines. The block keeps its own TestBed so the 42 existing tests keep their stubs untouched. Two of the new tests claimed more than they could observe and were rewritten: the force-render test asserted the registration form while its tab was selected, where it renders either way, and the close-button test asserted a tab index and a call count that were already at their expected values before the click. Both now start from a state the behaviour has to move, and the mutations that exposed them are red. No production file is touched.
Contributor
Automated Reviewer SuggestionsBased on the
|
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (2)
frontend/src/app/workspace/component/agent/agent-panel/agent-panel.component.spec.ts:883
- Avoid mutating the Vitest mock internals via
.mock.calls.length = 0; this is brittle and inconsistent with the rest of the file (which uses.mockClear()). Clear the mock using the supported API instead.
(service.activateAgent as unknown as { mock: { calls: unknown[] } }).mock.calls.length = 0;
frontend/src/app/workspace/component/agent/agent-panel/agent-panel.component.spec.ts:839
- This test reaches into child-component DOM internals (
.agent-chat-container,.model-card) and can become brittle if those templates change. It also starts on the registration tab, so depending on tab-pane retention behavior, the registration assertion may still be non-discriminating for[nzForceRender]. Prefer asserting on the presence of the child components themselves and make the registration tab inactive from the first render.
it("force-renders every tab body: the registration form and one chat per agent", () => {
service.agentList = [makeAgent("a"), makeAgent("b")];
createComponent();
// The agent tabs are unselected here, so their bodies exist only because they are
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7629 +/- ##
============================================
- Coverage 89.65% 89.11% -0.54%
+ Complexity 4397 4343 -54
============================================
Files 1177 1178 +1
Lines 46996 46872 -124
Branches 5268 5245 -23
============================================
- Hits 42136 41772 -364
- Misses 3094 3352 +258
+ Partials 1766 1748 -18
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
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.
What changes were proposed in this PR?
agent-panel.component.htmlreported 0 of 49 lines while its own.tssat at 133/133 with nothing missed. That pairing can only be attribution loss — this template is one of the better-tested in the frontend. The cause is #7458: the spec stubs its children out withTestBed.overrideComponent, and any override re-JITs the component from its decorator metadata, leaving the re-compiled template with no source map back to the.html.Adds a
describeblock that renders the component with its real children:agent-panel.component.htmlagent-panel.component.tsThe block keeps its own
TestBed, so the 42 existing tests keep their stubs and assertions untouched. Same remedy as merged PR #7535. Rendering the real children also incidentally lifts the child templates' own coverage.Verification
18 mutations applied and reverted, production diff empty each time.
Two of my new tests then turned out to claim more than they could observe, and both are fixed:
[nzForceRender]is set — flipping it tofalseleft the suite greenselectedTabIndexassertion was droppedselectedTabIndexwas already 0 andactivateAgentalready un-called, so both assertions held withevent.stopPropagation()deletedBoth exposing mutations — the registration tab losing
nzForceRender, and the close handler losingstopPropagation— are now red.That second one is worth spelling out: the mutation did turn the suite red before the fix, but the failure came from a pre-existing test elsewhere in the file, while the new test stayed green. A red suite is not evidence that the test under discussion pins anything.
Deliberately not included
Nothing in this PR pins the child components' internals; the assertions stay on this template's own structure and branches.
No production file is touched.
Any related issues, documentation, discussions?
Closes #7628
How was this PR tested?
9 new on top of the existing 42. Coverage re-measured by reverting the spec, running with
--coverage, and restoring.yarn format:cipasses.Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)