You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "mengw15 (via GitHub)" <gi...@apache.org> on 2026/08/15 04:20:12 UTC
[PR] test(frontend): cover the missing branches in four small files [texera]
mengw15 opened a new pull request, #7687:
URL: https://github.com/apache/texera/pull/7687
### What changes were proposed in this PR?
Takes the untaken branch on each conditional in four small files (15 new tests):
| File | Before | After |
| --- | --- | --- |
| `user-dataset-staged-objects-list.component.ts` | 31/32 lines, 17/22 branches | **32/32, 22/22** |
| `shared-model.ts` | 26/28 lines, 6/10 branches | **28/28, 10/10** |
| `ui-udf-parameters.component.ts` | 47/49 lines, 29/36 branches | **49/49, 36/36** |
| `landing-page.component.ts` | 45/45 lines, 16/21 branches | **45/45, 20/21** |
- **UserDatasetStagedObjectsList** — a missing `userMakeChangesEvent`, reverting
with no dataset id, the delete-failure notification, and the three
`getFileUploadTime` guards (no map, a path whose last segment is empty so the
`|| filePath` fallback runs, and a filename absent from the map).
- **SharedModel** — this file had no spec of its own, so this adds one: the room
suffix with and without a workflow id, the local `CoeditorState` published for
a signed-in user versus an anonymous one, `updateAwareness` in both modes,
`transact`, and each combination of `shouldConnect`/`wsconnected` in `destroy`.
The spec substitutes a `WebsocketProvider` double — the real one opens a socket
and schedules reconnects, which would put the network and a leaked timer inside
the test.
- **UiUdfParameters** — a parse error alongside the already-covered edit error and
an unrelated error that must be rethrown, a `fieldArray` supplied as a factory
function, a generated row that declares none of the expected columns (both
`if (!field) return` guards), a repeat populate that re-applies the disabled
state instead of re-wrapping the hook, and `trackByParameterName`'s `?? index`.
- **LandingPage** — a hub response without the requested action buckets plus an
enrichment that yields nothing, so all four `|| []` fallbacks run, and a
construction with no signed-in user.
`landing-page.component.ts` ends at 20/21: the remaining entry is a synthetic
branch the compiler/instrumenter attributes to the class-declaration line. The
behaviour behind it is covered — the new test drives the no-user path and asserts
`currentUid` stays undefined — but neither stubbing `getCurrentUser` nor clearing
the stub's user moves that counter.
No production code was changed.
### Any related issues, documentation, discussions?
Closes #7684.
### How was this PR tested?
`ng test --watch=false` over the four specs — 47 passed; the per-file line/branch numbers above come from the local lcov report. `eslint` and `prettier --check` clean. Failure path verified by breaking one new assertion in each of the four files: 4 failed / 43 passed, non-zero exit, then restored to green.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(frontend): cover the missing branches in SharedModel, staged objects, UDF parameters and landing page [texera]
Posted by "mengw15 (via GitHub)" <gi...@apache.org>.
mengw15 merged PR #7687:
URL: https://github.com/apache/texera/pull/7687
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(frontend): cover the missing branches in four small files [texera]
Posted by "mengw15 (via GitHub)" <gi...@apache.org>.
mengw15 commented on code in PR #7687:
URL: https://github.com/apache/texera/pull/7687#discussion_r3788523822
##########
frontend/src/app/hub/component/landing-page/landing-page.component.spec.ts:
##########
@@ -206,4 +206,28 @@ describe("LandingPageComponent", () => {
component.navigateToSearch("something-else");
expect(routerNavigateSpy).toHaveBeenCalledWith([HOME]);
});
+
+ it("leaves currentUid undefined when there is no signed-in user", () => {
+ // The stub seeds a user in its constructor; clear it before the component reads it.
+ userService.user = undefined;
+ build();
+ expect(component.isLogin).toBe(false);
+ expect(component.currentUid).toBeUndefined();
+ });
+
+ it("falls back to empty lists when the hub returns no buckets and the enrichment yields nothing", async () => {
+ // A tops response without the requested action keys exercises `topsMap[act] || []`,
+ // and an enrichment that resolves to nothing exercises the `|| []` guard on each
+ // of the three assigned lists (an empty array would be truthy and skip them).
+ hubServiceStub.getTops.mockReturnValue(of({}));
+ searchServiceStub.extendSearchResultsWithHubActivityInfo.mockReturnValue(of(undefined));
+ build();
+
+ await component.loadTops();
+
+ expect(searchServiceStub.extendSearchResultsWithHubActivityInfo).toHaveBeenCalledWith([], true, ["access"]);
+ expect(component.topLovedWorkflows).toEqual([]);
+ expect(component.topClonedWorkflows).toEqual([]);
+ expect(component.topLovedDatasets).toEqual([]);
+ });
Review Comment:
Agreed, and adopted the suggested split: one test drives `getTopLovedEntries` with a tops response that has no action keys (covering `topsMap[act] || []` while the search service keeps returning `DashboardEntry[]`), and a second stubs `getTopLovedEntries` to resolve maps without the keys so `loadTops`'s `|| []` fallbacks run. No stub returns `undefined` any more, so `SearchService`'s contract is respected. Branch coverage on the file is unchanged at 20/21.
Note the Autofix commit that applied this needed a formatting follow-up — its chained `.mockResolvedValueOnce()` call was a single line, which `prettier-eslint` reformats, so `format:ci` (and the frontend CI job) failed on it. That is fixed in the pushed version.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(frontend): cover the missing branches in four small files [texera]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #7687:
URL: https://github.com/apache/texera/pull/7687#discussion_r3788480172
##########
frontend/src/app/hub/component/landing-page/landing-page.component.spec.ts:
##########
@@ -206,4 +206,28 @@ describe("LandingPageComponent", () => {
component.navigateToSearch("something-else");
expect(routerNavigateSpy).toHaveBeenCalledWith([HOME]);
});
+
+ it("leaves currentUid undefined when there is no signed-in user", () => {
+ // The stub seeds a user in its constructor; clear it before the component reads it.
+ userService.user = undefined;
+ build();
+ expect(component.isLogin).toBe(false);
+ expect(component.currentUid).toBeUndefined();
+ });
+
+ it("falls back to empty lists when the hub returns no buckets and the enrichment yields nothing", async () => {
+ // A tops response without the requested action keys exercises `topsMap[act] || []`,
+ // and an enrichment that resolves to nothing exercises the `|| []` guard on each
+ // of the three assigned lists (an empty array would be truthy and skip them).
+ hubServiceStub.getTops.mockReturnValue(of({}));
+ searchServiceStub.extendSearchResultsWithHubActivityInfo.mockReturnValue(of(undefined));
+ build();
+
+ await component.loadTops();
+
+ expect(searchServiceStub.extendSearchResultsWithHubActivityInfo).toHaveBeenCalledWith([], true, ["access"]);
+ expect(component.topLovedWorkflows).toEqual([]);
+ expect(component.topClonedWorkflows).toEqual([]);
+ expect(component.topLovedDatasets).toEqual([]);
+ });
Review Comment:
This test forces `extendSearchResultsWithHubActivityInfo` to return `undefined`, but the real implementation always emits a `DashboardEntry[]` (it maps `items` and never returns `undefined`). That makes the test cover an unrealistic branch and can hide type/contract regressions.
Consider splitting the intent: (1) cover `topsMap[act] || []` by having `HubService.getTops` return `{}` while `extendSearchResultsWithHubActivityInfo` returns `[]`; and (2) cover `loadTops`’s `workflowEntries["like"] || []` fallbacks by mocking `getTopLovedEntries` to resolve missing keys, without violating `SearchService`’s contract.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(frontend): cover the missing branches in four small files [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7687:
URL: https://github.com/apache/texera/pull/7687#issuecomment-5300500268
<!-- texera-reviewer-suggestion -->
### Automated Reviewer Suggestions
Based on the `git blame` history of the changed files, we recommend the following reviewers:
- **Contributors with relevant context:** `@kunwp1`, `@Ma77Ball`, `@carloea2`
You can notify them by mentioning `@kunwp1`, `@Ma77Ball`, `@carloea2` in a comment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(frontend): cover the missing branches in four small files [texera]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #7687:
URL: https://github.com/apache/texera/pull/7687#issuecomment-5300539689
### :warning: JUnit XML file not found
The CLI was unable to find any JUnit XML files to upload.
For more help, visit our [troubleshooting guide](https://docs.codecov.com/docs/test-analytics#troubleshooting).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org