test(frontend): cover the time-travel poller and the feedback component - #7992
Open
aglinxinyuan wants to merge 1 commit into
Open
test(frontend): cover the time-travel poller and the feedback component#7992aglinxinyuan wants to merge 1 commit into
aglinxinyuan wants to merge 1 commit into
Conversation
+14 fully-covered lines, +9 branch arms, +7 functions. time-travel and both feedback files reach 100%; repeat-dnd goes 86.4% to 95.5%. feedback.component.html had ZERO of its two functions covered behind an 83% line figure -- the fifth time this campaign the function counter found what the line counter hid. time-travel's zero-hit lines were the whole ngOnInit timer lambda body. port-property-edit-frame was in scope and is absent, worth exactly zero: two of its three residual lines are dead Quill config. Chasing them found a real bug instead, reported not pinned -- the port-name editor's `key: 13` bindings are Quill 1 syntax that Quill 2 never dispatches, so Enter inserts a newline into the shared display name instead of closing the editor. repeat-dnd line 48 is left uncovered deliberately: it is the remove button's disabled binding, and issue apache#7431 records that the enclosing ngFor shadows `field` so it never disables. The assertion added there is on the add button. 28 mutations, 24 killed. The first pass claimed no survivors while eight were alive, and had two mutations sharing one number, so a kill was credited to the wrong row.
Contributor
Automated Reviewer SuggestionsBased on the
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7992 +/- ##
============================================
+ Coverage 93.21% 93.24% +0.03%
Complexity 4664 4664
============================================
Files 1175 1175
Lines 47646 47646
Branches 5319 5319
============================================
+ Hits 44413 44428 +15
+ Misses 1752 1739 -13
+ Partials 1481 1479 -2
*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:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
frontend/src/app/workspace/component/left-panel/time-travel/time-travel.component.spec.ts:279
- Same double-destroy risk here: this calls
fixture.destroy()inside the test and again inafterEach. Prefercomponent.ngOnDestroy()here to release the timer subscription within fakeAsync, leaving fixture teardown centralized inafterEach.
fixture.destroy();
frontend/src/app/workspace/component/left-panel/time-travel/time-travel.component.spec.ts:290
- Calling
fixture.destroy()here duplicates the suiteafterEachteardown and may raise a double-destroy exception. To simulate component teardown for the poller without destroying the fixture twice, callcomponent.ngOnDestroy()here instead (it triggers@UntilDestroycleanup) and keep the final fixture teardown inafterEach.
fixture.destroy();
| expect(widSpy).toHaveBeenCalledTimes(1); | ||
| expect(displaySpy).not.toHaveBeenCalled(); | ||
|
|
||
| fixture.destroy(); |
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?
Three small frontend components, measured from lcov with the same spec filter on both sides.
time-travel.component.tsfeedback.component.tsfeedback.component.htmlrepeat-dnd.component.htmlrepeat-dnd.component.ts(incidental)+14 fully-covered lines, +9 branch arms, +7 functions.
The template's function counter is the one to notice:
feedback.component.htmlwas at zero of two functions covered behind an 83% line figure. That is the fifth time in this campaign the function counter found what the line counter hid.time-travel.component.ts's zero-hit lines 84-88 were the entirengOnInittimer lambda body.port-property-edit-frame.component.tswas in scope and is absent. It is worth exactly zero: two of its three residual lines are dead Quill config and the third is unreachable in practice. No tests were added there and no mutations run.A production defect found while assessing it
Chasing those two Quill lines turned up a real bug rather than coverage: the port-name editor's keyboard bindings use
key: 13, which is Quill 1 syntax that Quill 2 never dispatches. So pressing Enter in the port-name editor inserts a newline into the shared display name instead of closing the editor. Reported, not pinned — pinning current behaviour would cement it.One arm left uncovered on purpose
repeat-dnd.component.htmlline 48 is the remove button's[disabled]="field.templateOptions?.disabled", and issue #7431 records that the enclosing*ngForshadows the outerfield, so it never disables. That arm is deliberately not pinned. The assertion added in that spec is on the add button, and it carries a comment explaining why an absenttemplateOptionsobject must not read as disabled.Verification
28 mutations, 24 killed, 4 recorded.
The first draft's headline was false: it claimed "survivors: NONE, every one of the 14 new tests proven by a mutation actually applied and run". Eight mutants survived it — seven found by review plus one found here (deleting an
[nzLoading]binding). All eight now die.Its survivor-to-mutation mapping was also wrong in one place: two separate mutations had been given the same number, so a kill was credited to the wrong row.
The four recorded non-kills are: the #7431 arm above (refused), an equivalent mutant (
[nzData]="[...feedbackList]"→[nzData]="feedbackList"— the spread produces an equal array), and two entries coveringport-property-edit-frame, where no tests were added.One further correction worth making:
feedback.component.htmlreaching 24/24 is not a quality claim on its own. At 24/24 the file is fully executed; what makes it constrained is the mutations, not the percentage.No production file is touched. The worktree used a real yarn install rather than a
node_modulesjunction.Any related issues, documentation, discussions?
Closes #7990
How was this PR tested?
yarn format:cipasses.frontend/junit.xmlandfrontend/coverage/are regenerated by every run and are not committed.Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)