This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-6691-1c411dedaad2bb8bd8fd4f5301843ae1f2f1c781 in repository https://gitbox.apache.org/repos/asf/texera.git
commit e2e441865c2ca766940133d8a24191234c3b4bfb Author: Matthew B. <[email protected]> AuthorDate: Wed Jul 22 21:51:23 2026 -0700 test(frontend): cover updateOrCreatePreset and preset guards (#6691) ### What changes were proposed in this PR? - Add branch coverage for PresetService.updateOrCreatePreset (identical, push, no-op, implicit-delete, replace). - Add tests for the isValidPreset and isValidPresetArray type guards. ### Any related issues, documentation, discussions? Closes: #6690 ### How was this PR tested? - Run: `cd frontend && node --max-old-space-size=8192 ./node_modules/nx/dist/bin/nx.js test gui --watch=false --include=src/app/workspace/service/preset/preset.service.spec.ts`, expect the suite passing. - Test-only change; no production code is modified. ### Was this PR authored or co-authored using generative AI tooling? Co-authored with Claude Opus 4.8 in compliance with ASF --------- Co-authored-by: Claude Opus 4.8 <[email protected]> Co-authored-by: Xinyuan Lin <[email protected]> --- .../service/preset/preset.service.spec.ts | 114 +++++++++++++++++++++ .../app/workspace/service/preset/preset.service.ts | 8 +- 2 files changed, 120 insertions(+), 2 deletions(-) diff --git a/frontend/src/app/workspace/service/preset/preset.service.spec.ts b/frontend/src/app/workspace/service/preset/preset.service.spec.ts index 4095418195..b71c976515 100644 --- a/frontend/src/app/workspace/service/preset/preset.service.spec.ts +++ b/frontend/src/app/workspace/service/preset/preset.service.spec.ts @@ -456,4 +456,118 @@ describe("PresetService", () => { }); }); }); + + describe("updateOrCreatePreset", () => { + // fetchKey is backed by a synchronous `of(...)`, so the subscribe body (and + // the savePresets write-through it triggers) runs before the call returns. + it("writes the stored preset list back unchanged when the original and replacement presets are identical", () => { + const stored: Preset[] = [{ presetProperty: "v1" }]; + userConfigStub.fetchKey.mockReturnValue(of(JSON.stringify(stored))); + + presetService.updateOrCreatePreset(presetType, presetTarget, { presetProperty: "x" }, { presetProperty: "x" }); + + // list is written back unchanged: neither pushed, replaced, nor spliced. + expect(userConfigStub.set).toHaveBeenCalledWith(presetDictKey, JSON.stringify(stored)); + }); + + it("appends the replacement when neither preset already exists", () => { + userConfigStub.fetchKey.mockReturnValue(of(JSON.stringify([{ presetProperty: "v1" }]))); + + presetService.updateOrCreatePreset( + presetType, + presetTarget, + { presetProperty: "missing" }, + { presetProperty: "v2" } + ); + + expect(userConfigStub.set).toHaveBeenCalledWith( + presetDictKey, + JSON.stringify([{ presetProperty: "v1" }, { presetProperty: "v2" }]) + ); + }); + + it("writes the stored preset list back unchanged when only the replacement preset already exists", () => { + const stored: Preset[] = [{ presetProperty: "v1" }, { presetProperty: "v2" }]; + userConfigStub.fetchKey.mockReturnValue(of(JSON.stringify(stored))); + + presetService.updateOrCreatePreset( + presetType, + presetTarget, + { presetProperty: "missing" }, + { presetProperty: "v2" } + ); + + expect(userConfigStub.set).toHaveBeenCalledWith(presetDictKey, JSON.stringify(stored)); + }); + + it("implicitly deletes a preset when both the original and the replacement exist", () => { + userConfigStub.fetchKey.mockReturnValue(of(JSON.stringify([{ presetProperty: "v1" }, { presetProperty: "v2" }]))); + + // Both presets are present (membership is checked deeply via isEqual), so the + // implicit-delete branch removes the original (v1) and leaves the replacement (v2). + presetService.updateOrCreatePreset(presetType, presetTarget, { presetProperty: "v1" }, { presetProperty: "v2" }); + + expect(userConfigStub.set).toHaveBeenCalledWith(presetDictKey, JSON.stringify([{ presetProperty: "v2" }])); + }); + + it("replaces the original preset in place when only the original exists", () => { + userConfigStub.fetchKey.mockReturnValue(of(JSON.stringify([{ presetProperty: "v1" }, { presetProperty: "v2" }]))); + + // The original exists (deep match) but the replacement does not, so the replace + // branch swaps the original (v1) for the replacement (v3) at its index. + presetService.updateOrCreatePreset(presetType, presetTarget, { presetProperty: "v1" }, { presetProperty: "v3" }); + + expect(userConfigStub.set).toHaveBeenCalledWith( + presetDictKey, + JSON.stringify([{ presetProperty: "v3" }, { presetProperty: "v2" }]) + ); + }); + }); + + describe("preset Ajv type guards", () => { + describe("isValidPreset", () => { + it("accepts an object whose values are all non-blank strings", () => { + expect(presetService.isValidPreset({ host: "localhost", table: "t1" })).toBe(true); + }); + + it("accepts an empty object (no properties to violate the schema)", () => { + expect(presetService.isValidPreset({})).toBe(true); + }); + + it("rejects a preset with a non-string value", () => { + expect(presetService.isValidPreset({ port: 5432 })).toBe(false); + }); + + it("rejects a preset with an empty-string value", () => { + expect(presetService.isValidPreset({ host: "" })).toBe(false); + }); + + it("rejects a preset whose value starts with whitespace", () => { + // the schema pattern ^\S.*$ requires the first character to be non-whitespace. + expect(presetService.isValidPreset({ host: " localhost" })).toBe(false); + }); + }); + + describe("isValidPresetArray", () => { + it("accepts an array of distinct valid presets", () => { + expect(presetService.isValidPresetArray([{ host: "a" }, { host: "b" }])).toBe(true); + }); + + it("accepts an empty array", () => { + expect(presetService.isValidPresetArray([])).toBe(true); + }); + + it("rejects an array containing duplicate presets (uniqueItems)", () => { + expect(presetService.isValidPresetArray([{ host: "a" }, { host: "a" }])).toBe(false); + }); + + it("rejects an array whose entry is not a valid preset", () => { + expect(presetService.isValidPresetArray([{ port: 5432 }])).toBe(false); + }); + + it("rejects a value that is not an array", () => { + expect(presetService.isValidPresetArray({ host: "a" } as any)).toBe(false); + }); + }); + }); }); diff --git a/frontend/src/app/workspace/service/preset/preset.service.ts b/frontend/src/app/workspace/service/preset/preset.service.ts index 9ba5c2a318..362063ded7 100644 --- a/frontend/src/app/workspace/service/preset/preset.service.ts +++ b/frontend/src/app/workspace/service/preset/preset.service.ts @@ -220,9 +220,13 @@ export class PresetService { // no modification: old preset doesn't exist to be updated, new preset already exists } else if (contains(presets, originalPreset) && contains(presets, replacementPreset)) { // implicit deletion by replacing original with existing preset - presets.splice(indexOf(presets, originalPreset), 1); + // deep-equality index: presets are freshly JSON-parsed, so reference-based indexOf would miss + presets.splice( + presets.findIndex(preset => isEqual(preset, originalPreset)), + 1 + ); } else { - presets[indexOf(presets, originalPreset)] = replacementPreset; + presets[presets.findIndex(preset => isEqual(preset, originalPreset))] = replacementPreset; } this.savePresets(type, target, presets, displayMessage, messageType); });
