From 2f315b596bc727509284f34f9ae6935939da3b2e Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Tue, 11 Jul 2023 17:55:04 +0200 Subject: [PATCH 1/6] Migrate FilesInput.vue to composition API + TS --- .../src/components/FilesDialog/FilesInput.vue | 95 +++++++++---------- 1 file changed, 45 insertions(+), 50 deletions(-) diff --git a/client/src/components/FilesDialog/FilesInput.vue b/client/src/components/FilesDialog/FilesInput.vue index 331b07c23216..5e969d64e152 100644 --- a/client/src/components/FilesDialog/FilesInput.vue +++ b/client/src/components/FilesDialog/FilesInput.vue @@ -1,55 +1,50 @@ - - - + + From eac14a7a205830e716154c675cab9babe753c419 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Wed, 12 Jul 2023 12:15:01 +0200 Subject: [PATCH 2/6] Migrate ExportForm.vue to composition API + TS Also migrate tests to TS and slightly increase coverage. --- ...{ExportForm.test.js => ExportForm.test.ts} | 55 +++++-- client/src/components/Common/ExportForm.vue | 136 +++++++++--------- 2 files changed, 108 insertions(+), 83 deletions(-) rename client/src/components/Common/{ExportForm.test.js => ExportForm.test.ts} (56%) diff --git a/client/src/components/Common/ExportForm.test.js b/client/src/components/Common/ExportForm.test.ts similarity index 56% rename from client/src/components/Common/ExportForm.test.js rename to client/src/components/Common/ExportForm.test.ts index 8afb419d7558..b613bd4a5cbe 100644 --- a/client/src/components/Common/ExportForm.test.js +++ b/client/src/components/Common/ExportForm.test.ts @@ -1,14 +1,12 @@ +import { getLocalVue } from "@tests/jest/helpers"; import { shallowMount } from "@vue/test-utils"; -import { getLocalVue } from "tests/jest/helpers"; import ExportForm from "./ExportForm.vue"; const localVue = getLocalVue(true); -jest.mock("components/JobStates/wait"); - describe("ExportForm.vue", () => { - let wrapper; + let wrapper: any; beforeEach(async () => { wrapper = shallowMount(ExportForm, { @@ -17,24 +15,41 @@ describe("ExportForm.vue", () => { }); }); - it("should render a form with export disable because inputs empty", async () => { - expect(wrapper.find(".export-button").exists()).toBeTruthy(); - expect(wrapper.find(".export-button").attributes("disabled")).toBeTruthy(); - expect(wrapper.vm.canExport).toBeFalsy(); + it("should render a form with export button disabled because inputs are empty", async () => { + expect(wrapper.vm.directory).toEqual(""); + expect(wrapper.vm.name).toEqual(""); + + expectExportButtonDisabled(); }); - it("should allow export when name and directory available", async () => { + it("should render a form with export button disabled because directory is empty", async () => { await wrapper.setData({ name: "export.tar.gz", + }); + + expectExportButtonDisabled(); + }); + + it("should render a form with export button disabled because name is empty", async () => { + await wrapper.setData({ directory: "gxfiles://", }); - expect(wrapper.vm.directory).toEqual("gxfiles://"); - expect(wrapper.vm.name).toEqual("export.tar.gz"); - expect(wrapper.vm.canExport).toBeTruthy(); + + expectExportButtonDisabled(); + }); + + it("should allow export when all inputs are defined", async () => { + await wrapper.setData({ + name: "export.tar.gz", + directory: "gxfiles://", + }); + + expectExportButtonEnabled(); }); it("should localize button text", async () => { - expect(wrapper.find(".export-button").text()).toBeLocalizationOf("Export"); + const newLocal = wrapper.find(".export-button").text(); + expect(newLocal).toBeLocalizationOf("Export"); }); it("should emit 'export' event with correct inputs on export button click", async () => { @@ -64,7 +79,17 @@ describe("ExportForm.vue", () => { await wrapper.find(".export-button").trigger("click"); - expect(wrapper.vm.directory).toBe(null); - expect(wrapper.vm.name).toBe(null); + expect(wrapper.vm.directory).toBe(""); + expect(wrapper.vm.name).toBe(""); }); + + function expectExportButtonDisabled() { + expect(wrapper.find(".export-button").exists()).toBeTruthy(); + expect(wrapper.find(".export-button").attributes("disabled")).toBeTruthy(); + } + + function expectExportButtonEnabled() { + expect(wrapper.find(".export-button").exists()).toBeTruthy(); + expect(wrapper.find(".export-button").attributes("disabled")).toBeFalsy(); + } }); diff --git a/client/src/components/Common/ExportForm.vue b/client/src/components/Common/ExportForm.vue index f571a6cb9fec..bc921c4f4515 100644 --- a/client/src/components/Common/ExportForm.vue +++ b/client/src/components/Common/ExportForm.vue @@ -1,73 +1,73 @@ + + - - From 00e4740ad800d3fcf56f59864d9e0cf52c0be0d0 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Wed, 12 Jul 2023 13:56:53 +0200 Subject: [PATCH 3/6] Fix selenium selector for history import --- client/src/utils/navigation/navigation.yml | 1 + test/integration_selenium/test_history_import_export_ftp.py | 1 + 2 files changed, 2 insertions(+) diff --git a/client/src/utils/navigation/navigation.yml b/client/src/utils/navigation/navigation.yml index bba88287c1b7..7fa348039a9e 100644 --- a/client/src/utils/navigation/navigation.yml +++ b/client/src/utils/navigation/navigation.yml @@ -493,6 +493,7 @@ last_export_record: history_import: selectors: radio_button_remote_files: '.history-import-component .fa-folder-open' + open_files_dialog: '.history-import-component .directory-form-input' import_button: '.import-button' running: '.history-import-component .loading-icon' success_message: '.history-import-component .alert-success' diff --git a/test/integration_selenium/test_history_import_export_ftp.py b/test/integration_selenium/test_history_import_export_ftp.py index 89dc1941bc24..ceac83751142 100644 --- a/test/integration_selenium/test_history_import_export_ftp.py +++ b/test/integration_selenium/test_history_import_export_ftp.py @@ -71,6 +71,7 @@ def test_history_import_export(self): gx_selenium_context.components.histories.import_button.wait_for_and_click() history_import = gx_selenium_context.components.history_import history_import.radio_button_remote_files.wait_for_and_click() + history_import.open_files_dialog.wait_for_and_click() files_dialog.ftp_label.wait_for_and_click() files_dialog.row(uri="gxftp://my_export.tar.gz").wait_for_and_click() From 6e791578d65210c41b9246fc5a4ef871d403b30b Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Mon, 17 Jul 2023 13:42:51 +0200 Subject: [PATCH 4/6] Use empty string for FilesInput value An empty string is treated like a null-string anyway but doesn't require to deal with extra typing cases. --- client/src/components/HistoryImport.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/src/components/HistoryImport.vue b/client/src/components/HistoryImport.vue index a03782904391..f0ebe45fa517 100644 --- a/client/src/components/HistoryImport.vue +++ b/client/src/components/HistoryImport.vue @@ -125,7 +125,7 @@ export default { initializing: true, importType: "externalUrl", sourceFile: null, - sourceRemoteFilesUri: null, + sourceRemoteFilesUri: "", errorMessage: null, waitingOnJob: false, complete: false, From 236b17b858778f1feabc1f4be9f2e8f78e2c9de8 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Wed, 19 Jul 2023 11:06:08 +0200 Subject: [PATCH 5/6] Restrict mode in FilesInput to valid values --- client/src/components/FilesDialog/FilesInput.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/src/components/FilesDialog/FilesInput.vue b/client/src/components/FilesDialog/FilesInput.vue index 5e969d64e152..11a1abcd5d8d 100644 --- a/client/src/components/FilesDialog/FilesInput.vue +++ b/client/src/components/FilesDialog/FilesInput.vue @@ -6,7 +6,7 @@ import { filesDialog } from "@/utils/data"; interface Props { value: string; - mode?: string; + mode?: "file" | "directory"; requireWritable?: boolean; } From 9b20f77546a7067e5cc3a5c6fa9a40a26f1081e0 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Wed, 19 Jul 2023 11:59:44 +0200 Subject: [PATCH 6/6] Rewrite ExportForm tests without exposing internals --- .../src/components/Common/ExportForm.test.ts | 52 +++++++++---------- client/src/components/Common/ExportForm.vue | 9 ---- 2 files changed, 24 insertions(+), 37 deletions(-) diff --git a/client/src/components/Common/ExportForm.test.ts b/client/src/components/Common/ExportForm.test.ts index b613bd4a5cbe..d2ae7d2422df 100644 --- a/client/src/components/Common/ExportForm.test.ts +++ b/client/src/components/Common/ExportForm.test.ts @@ -1,7 +1,8 @@ import { getLocalVue } from "@tests/jest/helpers"; -import { shallowMount } from "@vue/test-utils"; +import { mount } from "@vue/test-utils"; import ExportForm from "./ExportForm.vue"; +import FilesInput from "@/components/FilesDialog/FilesInput.vue"; const localVue = getLocalVue(true); @@ -9,40 +10,33 @@ describe("ExportForm.vue", () => { let wrapper: any; beforeEach(async () => { - wrapper = shallowMount(ExportForm, { + wrapper = mount(ExportForm, { propsData: {}, localVue, }); }); it("should render a form with export button disabled because inputs are empty", async () => { - expect(wrapper.vm.directory).toEqual(""); - expect(wrapper.vm.name).toEqual(""); - expectExportButtonDisabled(); }); it("should render a form with export button disabled because directory is empty", async () => { - await wrapper.setData({ - name: "export.tar.gz", - }); + const newValue = "export.tar.gz"; + await setNameInput(newValue); expectExportButtonDisabled(); }); it("should render a form with export button disabled because name is empty", async () => { - await wrapper.setData({ - directory: "gxfiles://", - }); + const newValue = "gxfiles://"; + await setDirectoryInput(newValue); expectExportButtonDisabled(); }); it("should allow export when all inputs are defined", async () => { - await wrapper.setData({ - name: "export.tar.gz", - directory: "gxfiles://", - }); + await setNameInput("export.tar.gz"); + await setDirectoryInput("gxfiles://"); expectExportButtonEnabled(); }); @@ -53,10 +47,8 @@ describe("ExportForm.vue", () => { }); it("should emit 'export' event with correct inputs on export button click", async () => { - await wrapper.setData({ - name: "export.tar.gz", - directory: "gxfiles://", - }); + await setNameInput("export.tar.gz"); + await setDirectoryInput("gxfiles://"); expect(wrapper.emitted()).not.toHaveProperty("export"); await wrapper.find(".export-button").trigger("click"); @@ -66,21 +58,16 @@ describe("ExportForm.vue", () => { expect(wrapper.emitted()["export"][0][1]).toBe("export.tar.gz"); }); - it("should clear the inputs after export when clearInputAfterExport is enabled", async () => { + it("should clear the inputs (hence disabling export) after export when clearInputAfterExport is enabled", async () => { await wrapper.setProps({ clearInputAfterExport: true, }); - await wrapper.setData({ - name: "export.tar.gz", - directory: "gxfiles://", - }); - expect(wrapper.vm.directory).toEqual("gxfiles://"); - expect(wrapper.vm.name).toEqual("export.tar.gz"); + await setNameInput("export.tar.gz"); + await setDirectoryInput("gxfiles://"); await wrapper.find(".export-button").trigger("click"); - expect(wrapper.vm.directory).toBe(""); - expect(wrapper.vm.name).toBe(""); + expectExportButtonDisabled(); }); function expectExportButtonDisabled() { @@ -92,4 +79,13 @@ describe("ExportForm.vue", () => { expect(wrapper.find(".export-button").exists()).toBeTruthy(); expect(wrapper.find(".export-button").attributes("disabled")).toBeFalsy(); } + + async function setNameInput(newValue: string) { + const nameInput = wrapper.find("#name"); + await nameInput.setValue(newValue); + } + + async function setDirectoryInput(newValue: string) { + await wrapper.findComponent(FilesInput).vm.$emit("input", newValue); + } }); diff --git a/client/src/components/Common/ExportForm.vue b/client/src/components/Common/ExportForm.vue index bc921c4f4515..993b5c641a14 100644 --- a/client/src/components/Common/ExportForm.vue +++ b/client/src/components/Common/ExportForm.vue @@ -38,15 +38,6 @@ const doExport = () => { name.value = ""; } }; - -// This is required to make the component testable with script setup, -// otherwise we cannot access the component's data. -// We probably can remove this once we switch to Vue 3 or other testing framework. -defineExpose({ - directory, - name, - canExport, -});