test(frontend): cover joint-ui.service surface and remove dead code#5196
Open
Yicong-Huang wants to merge 3 commits into
Open
test(frontend): cover joint-ui.service surface and remove dead code#5196Yicong-Huang wants to merge 3 commits into
Yicong-Huang wants to merge 3 commits into
Conversation
…lock The spec opened with 60 commented test lines and an `it.todo` placeholder ahead of the real tests. Five of the seven SUT methods the commented tests called have been removed from `JointUIService` over the years (`getJointOperatorStatusTooltipElement`, `changeOperatorStates`, `changeOperatorTooltipInfo`, `showToolTip`, `hideTooltip`) — those tests have not been runnable in a long time. Drop the dead commented block and the now-misleading `it.todo`. Add direct coverage for the public surface the spec was leaving behind: - static helpers `getOperatorFillColor`, `getOperatorCacheDisplayText`, `getOperatorCacheIcon`, `getOperatorViewResultIcon`, `getJointLinkCell`, `getJointUserPointerName`, `getJointUserPointerCell`; - joint-paper-bound methods `changeOperatorColor`, `changeOperatorState` (all six state branches), `foldOperatorDetails`/`unfoldOperatorDetails`, `showAgentActionLabel`/`hideAgentActionLabel` (including the missing-model no-op paths), `getCommentElement`; - `getJointOperatorElement`'s "unknown operator type" throw branch. Coverage of `joint-ui.service.ts` rises from 34 % (75/220) to 70 % (155/220). Test count goes from 11 + 1 todo to 42 live tests. Closes apache#5195
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5196 +/- ##
============================================
+ Coverage 45.39% 45.63% +0.23%
Complexity 2218 2218
============================================
Files 1042 1042
Lines 39989 39973 -16
Branches 4260 4251 -9
============================================
+ Hits 18152 18240 +88
+ Misses 20720 20617 -103
+ Partials 1117 1116 -1
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…ntOperatorElement The first pass at this spec landed `JointUIService` at 70 % line coverage; the next ROI tier is the port-iteration branches inside `changeOperatorState`, the full body of `changeOperatorStatistics`, and `getJointOperatorElement`'s happy path. Add direct tests for each: - `getJointOperatorElement`: builds an element with the predicate's `operatorID` + z=1; wires ports through `addPort`; emits the add/remove port-button markup when the dynamic-port flags are true; routes `customDisplayName` into `.texera-operator-name/text`. - `changeOperatorState`: when `getPorts()` returns real ports, the forEach loop reaches `portProp(...)` for every port group. - `changeOperatorDisableStatus`, `changeOperatorViewResultStatus`, `changeOperatorReuseCacheStatus`: small attr-setters whose default and asset-path branches were both uncovered. - `changeOperatorStatistics`: covers the undefined-statistics fallback, the per-port label rewrites driven by `inputPortMetrics` / `outputPortMetrics`, and the worker-count formatting. Line coverage of `joint-ui.service.ts` rises from 70 % (155/220) to 98 % (215/220). Remaining uncovered lines are the unreachable `if (!element) return` branch in `unfoldOperatorDetails` and a few guard expressions inside the same loop.
Pushing line coverage to 98% in the previous commits surfaced four genuinely dead spots in the SUT itself, all of them stale residue from earlier refactors: 1. `getCustomOperatorStatusTooltipStyleAttrs()` — a static method returning a tooltip style config. `grep` finds zero callers anywhere in the codebase; the tooltip element this paired with (`getJointOperatorStatusTooltipElement`) was removed long ago. 2. `changeOperatorStatistics` input-port loop: `rawAttrs`, `oldText`, `originalName` are all derived from each other but never read — the port label text comes from `count.toLocaleString()`, not the originalName. Same dead chain. 3. The output-port loop carries the identical dead chain. 4. `unfoldOperatorDetails`'s tail: the method reaches for the joint model a second time, casts it to `joint.shapes.devs.Model`, and returns early when null. There is nothing past the early return, so the guard is a no-op. After deletion: SUT shrinks from 220 instrumented lines to 204; v8 line coverage reports 203/204 (the one remaining "unhit" line is the `export class …` declaration itself, an instrumentation artifact).
Yicong-Huang
changed the title
Yicong-Huang
changed the title
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
2 participants
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?
joint-ui.service.spec.tshad ~60 commented test lines plus anit.todoplaceholder; the commented tests called methods that have since been removed fromJointUIService, so they're unrevivable. Drop them and add real tests for the public surface the spec was leaving unverified — static helpers, the joint-paper-bound state/color/visibility setters, the port-iteration loops,getJointOperatorElement, andchangeOperatorStatistics.Writing those tests made four pieces of stale
joint-ui.service.tsitself obvious: a static tooltip-style helper with zero callers (its paired element was removed years ago), two deadoriginalNamechains insidechangeOperatorStatistics(computed, never read), and aif (!element) returnguard at the very end ofunfoldOperatorDetails(no-op — nothing follows it). Removed.Any related issues, documentation, discussions?
Closes #5195. Same shape as #5194.
How was this PR tested?
yarn ng test --watch=false --include=…runs 56 tests, all green.yarn lintandyarn format:ciboth clean.Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Opus 4.7