This is an automated email from the ASF dual-hosted git repository.
hansva pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/hop.git
The following commit(s) were added to refs/heads/main by this push:
new eadd814257 disabled buttons for unit tests and code hardening, fixes
#6406 (#6407)
eadd814257 is described below
commit eadd814257a93a2a93b5345da1d28358992c4306
Author: Hans Van Akelyen <[email protected]>
AuthorDate: Fri Jan 16 11:32:36 2026 +0100
disabled buttons for unit tests and code hardening, fixes #6406 (#6407)
cleanup dialog a bit
---
.../apache/hop/testing/gui/TestingGuiPlugin.java | 100 +++++++++++++++++++--
.../xp/UpdateUnitTestButtonsExtensionPoint.java | 53 +++++++++++
.../hop/ui/testing/PipelineUnitTestEditor.java | 55 ++++++------
.../hop/ui/core/gui/GuiCompositeWidgets.java | 15 +++-
.../apache/hop/ui/core/gui/GuiToolbarWidgets.java | 4 +
5 files changed, 191 insertions(+), 36 deletions(-)
diff --git
a/plugins/misc/testing/src/main/java/org/apache/hop/testing/gui/TestingGuiPlugin.java
b/plugins/misc/testing/src/main/java/org/apache/hop/testing/gui/TestingGuiPlugin.java
index d4efed5fc9..4ba0658774 100644
---
a/plugins/misc/testing/src/main/java/org/apache/hop/testing/gui/TestingGuiPlugin.java
+++
b/plugins/misc/testing/src/main/java/org/apache/hop/testing/gui/TestingGuiPlugin.java
@@ -112,6 +112,8 @@ public class TestingGuiPlugin {
private static TestingGuiPlugin instance = null;
+ private static final ILogChannel log = LogChannel.GENERAL;
+
public TestingGuiPlugin() {
// Do nothing
}
@@ -871,6 +873,9 @@ public class TestingGuiPlugin {
// Update the GUI
//
pipelineGraph.updateGui();
+
+ // Enable/disable buttons based on selection
+ enableUnitTestButtons();
} catch (Exception e) {
new ErrorDialog(
hopGui.getShell(),
@@ -892,7 +897,30 @@ public class TestingGuiPlugin {
if (pipelineMeta == null) {
return;
}
- PipelineUnitTest unitTest = getCurrentUnitTest(pipelineMeta);
+ Combo combo = getUnitTestsCombo();
+ if (combo == null) {
+ return;
+ }
+ if (StringUtils.isEmpty(combo.getText())) {
+ return;
+ }
+
+ String unitTestName = combo.getText();
+
+ // Load the unit test to verify it exists
+ PipelineUnitTest unitTest = null;
+ try {
+ IHopMetadataSerializer<PipelineUnitTest> testSerializer =
+ hopGui.getMetadataProvider().getSerializer(PipelineUnitTest.class);
+ unitTest = testSerializer.load(unitTestName);
+ } catch (Exception e) {
+ log.logError("Error loading unit test: " + unitTestName, e);
+ return;
+ }
+
+ if (unitTest == null || Utils.isEmpty(unitTest.getName())) {
+ return;
+ }
MetadataManager<PipelineUnitTest> manager =
new MetadataManager<>(
@@ -900,12 +928,25 @@ public class TestingGuiPlugin {
hopGui.getMetadataProvider(),
PipelineUnitTest.class,
hopGui.getShell());
- if (unitTest != null
- && !Utils.isEmpty(unitTest.getName())
- && manager.editMetadata(unitTest.getName())) {
+ if (manager.editMetadata(unitTest.getName())) {
// Activate the test
refreshUnitTestsList();
- selectUnitTest(pipelineMeta, unitTest);
+
+ // Reload the unit test and select it
+ try {
+ IHopMetadataSerializer<PipelineUnitTest> testSerializer =
+ hopGui.getMetadataProvider().getSerializer(PipelineUnitTest.class);
+ PipelineUnitTest reloadedTest = testSerializer.load(unitTestName);
+ if (reloadedTest != null) {
+ selectUnitTest(pipelineMeta, reloadedTest);
+ }
+ } catch (Exception e) {
+ // Log but don't fail
+ log.logError("Error reloading unit test after edit", e);
+ }
+
+ // Enable/disable buttons based on selection
+ enableUnitTestButtons();
}
}
@@ -939,6 +980,9 @@ public class TestingGuiPlugin {
// Activate the test
refreshUnitTestsList();
selectUnitTest(pipelineMeta, test);
+
+ // Enable/disable buttons based on selection
+ enableUnitTestButtons();
}
}
@@ -995,6 +1039,9 @@ public class TestingGuiPlugin {
testSerializer.delete(pipelineUnitTest.getName());
refreshUnitTestsList();
+
+ // Enable/disable buttons based on selection
+ enableUnitTestButtons();
} catch (Exception e) {
new ErrorDialog(
hopGui.getShell(),
@@ -1017,12 +1064,52 @@ public class TestingGuiPlugin {
return null;
}
+ /**
+ * Enable or disable the unit test buttons (Edit, Detach, Delete) based on
whether a unit test is
+ * selected.
+ */
+ public void enableUnitTestButtons() {
+ HopGuiPipelineGraph pipelineGraph = HopGui.getActivePipelineGraph();
+ if (pipelineGraph == null) {
+ return;
+ }
+
+ // Check if toolbar widgets are available
+ if (pipelineGraph.getToolBarWidgets() == null) {
+ return;
+ }
+
+ Combo combo = getUnitTestsCombo();
+ boolean hasSelection = combo != null &&
!StringUtils.isEmpty(combo.getText());
+
+ if (log.isDebug()) {
+ log.logDebug(
+ "Enabling unit test buttons: hasSelection="
+ + hasSelection
+ + ", comboText="
+ + (combo != null ? combo.getText() : "null"));
+ }
+
+ pipelineGraph
+ .getToolBarWidgets()
+ .enableToolbarItem(ID_TOOLBAR_ITEM_UNIT_TEST_EDIT, hasSelection);
+ pipelineGraph
+ .getToolBarWidgets()
+ .enableToolbarItem(ID_TOOLBAR_ITEM_UNIT_TEST_DETACH, hasSelection);
+ pipelineGraph
+ .getToolBarWidgets()
+ .enableToolbarItem(ID_TOOLBAR_ITEM_UNIT_TESTS_DELETE, hasSelection);
+ }
+
public static void refreshUnitTestsList() {
HopGuiPipelineGraph pipelineGraph = HopGui.getActivePipelineGraph();
if (pipelineGraph == null) {
return;
}
pipelineGraph.getToolBarWidgets().refreshComboItemList(ID_TOOLBAR_UNIT_TESTS_COMBO);
+
+ // Update button states after refresh
+ getInstance().enableUnitTestButtons();
}
public static void selectUnitTestInList(String name) {
@@ -1178,6 +1265,9 @@ public class TestingGuiPlugin {
// Update the pipeline graph
hopGui.getActiveFileTypeHandler().updateGui();
}
+
+ // Enable/disable buttons based on selection
+ enableUnitTestButtons();
} catch (Exception e) {
new ErrorDialog(
hopGui.getShell(),
diff --git
a/plugins/misc/testing/src/main/java/org/apache/hop/testing/xp/UpdateUnitTestButtonsExtensionPoint.java
b/plugins/misc/testing/src/main/java/org/apache/hop/testing/xp/UpdateUnitTestButtonsExtensionPoint.java
new file mode 100644
index 0000000000..e3a5f63bbb
--- /dev/null
+++
b/plugins/misc/testing/src/main/java/org/apache/hop/testing/xp/UpdateUnitTestButtonsExtensionPoint.java
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hop.testing.xp;
+
+import org.apache.hop.core.exception.HopException;
+import org.apache.hop.core.extension.ExtensionPoint;
+import org.apache.hop.core.extension.IExtensionPoint;
+import org.apache.hop.core.logging.ILogChannel;
+import org.apache.hop.core.variables.IVariables;
+import org.apache.hop.testing.gui.TestingGuiPlugin;
+import org.apache.hop.ui.hopgui.HopGui;
+import org.apache.hop.ui.hopgui.file.pipeline.HopGuiPipelineGraph;
+import org.eclipse.swt.widgets.Display;
+
+@ExtensionPoint(
+ extensionPointId = "HopGuiPipelineGraphUpdateGui",
+ id = "UpdateUnitTestButtonsUpdateGuiExtensionPoint",
+ description = "Enable/disable unit test toolbar buttons based on whether a
test is selected")
+/**
+ * This extension point is called whenever the pipeline graph GUI is updated.
It ensures the unit
+ * test toolbar buttons (Edit, Detach, Delete) are enabled or disabled based
on whether a unit test
+ * is currently selected.
+ */
+public class UpdateUnitTestButtonsExtensionPoint implements
IExtensionPoint<HopGuiPipelineGraph> {
+
+ @Override
+ public void callExtensionPoint(
+ ILogChannel log, IVariables variables, HopGuiPipelineGraph pipelineGraph)
+ throws HopException {
+
+ // Update the unit test button states
+ // Use asyncExec to ensure this runs after any async combo population
+ Display display = HopGui.getInstance().getDisplay();
+ if (display != null && !display.isDisposed()) {
+ display.asyncExec(() ->
TestingGuiPlugin.getInstance().enableUnitTestButtons());
+ }
+ }
+}
diff --git
a/plugins/misc/testing/src/main/java/org/apache/hop/ui/testing/PipelineUnitTestEditor.java
b/plugins/misc/testing/src/main/java/org/apache/hop/ui/testing/PipelineUnitTestEditor.java
index a74b0f1177..efafe5c003 100644
---
a/plugins/misc/testing/src/main/java/org/apache/hop/ui/testing/PipelineUnitTestEditor.java
+++
b/plugins/misc/testing/src/main/java/org/apache/hop/ui/testing/PipelineUnitTestEditor.java
@@ -99,7 +99,7 @@ public class PipelineUnitTestEditor extends
MetadataEditor<PipelineUnitTest> {
wName = new Text(parent, SWT.SINGLE | SWT.LEFT | SWT.BORDER);
PropsUi.setLook(wName);
FormData fdName = new FormData();
- fdName.top = new FormAttachment(wlName, 0, SWT.CENTER);
+ fdName.top = new FormAttachment(0, margin);
fdName.left = new FormAttachment(middle, 0);
fdName.right = new FormAttachment(100, 0);
wName.setLayoutData(fdName);
@@ -118,7 +118,7 @@ public class PipelineUnitTestEditor extends
MetadataEditor<PipelineUnitTest> {
wDescription = new Text(parent, SWT.SINGLE | SWT.LEFT | SWT.BORDER);
PropsUi.setLook(wDescription);
FormData fdDescription = new FormData();
- fdDescription.top = new FormAttachment(wlDescription, 0, SWT.CENTER);
+ fdDescription.top = new FormAttachment(lastControl, margin);
fdDescription.left = new FormAttachment(middle, 0);
fdDescription.right = new FormAttachment(100, 0);
wDescription.setLayoutData(fdDescription);
@@ -137,7 +137,7 @@ public class PipelineUnitTestEditor extends
MetadataEditor<PipelineUnitTest> {
wTestType = new Combo(parent, SWT.SINGLE | SWT.LEFT | SWT.BORDER);
PropsUi.setLook(wTestType);
FormData fdTestType = new FormData();
- fdTestType.top = new FormAttachment(wlTestType, 0, SWT.CENTER);
+ fdTestType.top = new FormAttachment(lastControl, margin);
fdTestType.left = new FormAttachment(middle, 0);
fdTestType.right = new FormAttachment(100, 0);
wTestType.setLayoutData(fdTestType);
@@ -153,25 +153,25 @@ public class PipelineUnitTestEditor extends
MetadataEditor<PipelineUnitTest> {
FormData fdlPipelineFilename = new FormData();
fdlPipelineFilename.top = new FormAttachment(lastControl, margin);
fdlPipelineFilename.left = new FormAttachment(0, 0);
- fdlPipelineFilename.right = new FormAttachment(middle, 0);
+ fdlPipelineFilename.right = new FormAttachment(middle, -margin);
wlPipelineFilename.setLayoutData(fdlPipelineFilename);
Button wbPipelineFilename = new Button(parent, SWT.PUSH);
PropsUi.setLook(wbPipelineFilename);
wbPipelineFilename.setText(BaseMessages.getString(PKG,
"PipelineUnitTestDialog.Button.Browse"));
FormData fdbPipelineFilename = new FormData();
- fdbPipelineFilename.right = new FormAttachment(99, 0);
- fdbPipelineFilename.top = new FormAttachment(wlPipelineFilename, 0,
SWT.CENTER);
+ fdbPipelineFilename.right = new FormAttachment(100, 0);
+ fdbPipelineFilename.top = new FormAttachment(lastControl, margin);
wbPipelineFilename.setLayoutData(fdbPipelineFilename);
wbPipelineFilename.addListener(SWT.Selection,
this::browsePipelineFilename);
wPipelineFilename = new Text(parent, SWT.SINGLE | SWT.LEFT | SWT.BORDER);
PropsUi.setLook(wPipelineFilename);
FormData fdPipelineFilename = new FormData();
- fdPipelineFilename.top = new FormAttachment(wlPipelineFilename, 0,
SWT.CENTER);
- fdPipelineFilename.left = new FormAttachment(middle, margin);
+ fdPipelineFilename.top = new FormAttachment(lastControl, margin);
+ fdPipelineFilename.left = new FormAttachment(middle, 0);
fdPipelineFilename.right = new FormAttachment(wbPipelineFilename, -margin);
wPipelineFilename.setLayoutData(fdPipelineFilename);
- lastControl = wPipelineFilename;
+ lastControl = wbPipelineFilename;
// The optional filename of the test result...
//
@@ -186,7 +186,7 @@ public class PipelineUnitTestEditor extends
MetadataEditor<PipelineUnitTest> {
wFilename = new TextVar(manager.getVariables(), parent, SWT.SINGLE |
SWT.LEFT | SWT.BORDER);
PropsUi.setLook(wFilename);
FormData fdFilename = new FormData();
- fdFilename.top = new FormAttachment(wlFilename, 0, SWT.CENTER);
+ fdFilename.top = new FormAttachment(lastControl, margin);
fdFilename.left = new FormAttachment(middle, 0);
fdFilename.right = new FormAttachment(100, 0);
wFilename.setLayoutData(fdFilename);
@@ -200,28 +200,28 @@ public class PipelineUnitTestEditor extends
MetadataEditor<PipelineUnitTest> {
FormData fdlBasePath = new FormData();
fdlBasePath.top = new FormAttachment(lastControl, margin);
fdlBasePath.left = new FormAttachment(0, 0);
- fdlBasePath.right = new FormAttachment(middle, 0);
+ fdlBasePath.right = new FormAttachment(middle, -margin);
wlBasePath.setLayoutData(fdlBasePath);
Button wbBasePath = new Button(parent, SWT.PUSH);
PropsUi.setLook(wbBasePath);
wbBasePath.setText(BaseMessages.getString(PKG,
"PipelineUnitTestDialog.Button.Browse"));
FormData fdbBasePath = new FormData();
- fdbBasePath.right = new FormAttachment(99, 0);
- fdbBasePath.top = new FormAttachment(wlBasePath, 0, SWT.CENTER);
+ fdbBasePath.right = new FormAttachment(100, 0);
+ fdbBasePath.top = new FormAttachment(lastControl, margin);
wbBasePath.setLayoutData(fdbBasePath);
wbBasePath.addListener(SWT.Selection, this::browseTestPathDir);
wBasePath = new TextVar(manager.getVariables(), parent, SWT.SINGLE |
SWT.LEFT | SWT.BORDER);
PropsUi.setLook(wBasePath);
FormData fdBasePath = new FormData();
- fdBasePath.top = new FormAttachment(wbBasePath, 0, SWT.CENTER);
- fdBasePath.left = new FormAttachment(middle, margin);
+ fdBasePath.top = new FormAttachment(lastControl, margin);
+ fdBasePath.left = new FormAttachment(middle, 0);
fdBasePath.right = new FormAttachment(wbBasePath, -margin);
wBasePath.setLayoutData(fdBasePath);
- lastControl = wBasePath;
+ lastControl = wbBasePath;
- // The base path for relative test path resolution
+ // Auto-open checkbox
//
Label wlAutoOpen = new Label(parent, SWT.RIGHT);
PropsUi.setLook(wlAutoOpen);
@@ -236,21 +236,20 @@ public class PipelineUnitTestEditor extends
MetadataEditor<PipelineUnitTest> {
FormData fdAutoOpen = new FormData();
fdAutoOpen.top = new FormAttachment(wlAutoOpen, 0, SWT.CENTER);
fdAutoOpen.left = new FormAttachment(middle, 0);
- fdAutoOpen.right = new FormAttachment(100, 0);
wAutoOpen.setLayoutData(fdAutoOpen);
lastControl = wAutoOpen;
// The list of database replacements in the unit test pipeline
//
- Label wlFieldMapping = new Label(parent, SWT.NONE);
- wlFieldMapping.setText(
+ Label wlDbReplacements = new Label(parent, SWT.NONE);
+ wlDbReplacements.setText(
BaseMessages.getString(PKG,
"PipelineUnitTestDialog.DbReplacements.Label"));
- PropsUi.setLook(wlFieldMapping);
- FormData fdlUpIns = new FormData();
- fdlUpIns.left = new FormAttachment(0, 0);
- fdlUpIns.top = new FormAttachment(lastControl, 3 * margin);
- wlFieldMapping.setLayoutData(fdlUpIns);
- lastControl = wlFieldMapping;
+ PropsUi.setLook(wlDbReplacements);
+ FormData fdlDbReplacements = new FormData();
+ fdlDbReplacements.left = new FormAttachment(0, 0);
+ fdlDbReplacements.top = new FormAttachment(lastControl, 2 * margin);
+ wlDbReplacements.setLayoutData(fdlDbReplacements);
+ lastControl = wlDbReplacements;
// the database replacements
//
@@ -294,7 +293,7 @@ public class PipelineUnitTestEditor extends
MetadataEditor<PipelineUnitTest> {
fdDbReplacements.left = new FormAttachment(0, 0);
fdDbReplacements.top = new FormAttachment(lastControl, margin);
fdDbReplacements.right = new FormAttachment(100, 0);
- fdDbReplacements.bottom = new FormAttachment(lastControl, 250);
+ fdDbReplacements.bottom = new FormAttachment(50, -margin);
wDbReplacements.setLayoutData(fdDbReplacements);
lastControl = wDbReplacements;
@@ -304,7 +303,7 @@ public class PipelineUnitTestEditor extends
MetadataEditor<PipelineUnitTest> {
PropsUi.setLook(wlVariableValues);
FormData fdlVariableValues = new FormData();
fdlVariableValues.left = new FormAttachment(0, 0);
- fdlVariableValues.top = new FormAttachment(lastControl, margin);
+ fdlVariableValues.top = new FormAttachment(lastControl, 2 * margin);
wlVariableValues.setLayoutData(fdlVariableValues);
lastControl = wlVariableValues;
diff --git
a/ui/src/main/java/org/apache/hop/ui/core/gui/GuiCompositeWidgets.java
b/ui/src/main/java/org/apache/hop/ui/core/gui/GuiCompositeWidgets.java
index c0847937c8..5eb69fb899 100644
--- a/ui/src/main/java/org/apache/hop/ui/core/gui/GuiCompositeWidgets.java
+++ b/ui/src/main/java/org/apache/hop/ui/core/gui/GuiCompositeWidgets.java
@@ -642,8 +642,12 @@ public class GuiCompositeWidgets {
// New layout: control goes on the right, aligned with the row below
the label
fdControl.top = new FormAttachment(label, PropsUi.getMargin() / 2);
} else {
- // Old layout: control aligned with label (center)
- fdControl.top = new FormAttachment(label, 0, SWT.CENTER);
+ // Old layout: control aligned with lastControl to maintain proper
vertical spacing
+ if (lastControl != null) {
+ fdControl.top = new FormAttachment(lastControl, PropsUi.getMargin());
+ } else {
+ fdControl.top = new FormAttachment(0, PropsUi.getMargin());
+ }
}
} else {
if (lastControl != null) {
@@ -681,7 +685,12 @@ public class GuiCompositeWidgets {
} else {
fdControl.right = new FormAttachment(rightControl, -5);
}
- fdControl.top = new FormAttachment(label, 0, SWT.CENTER);
+ // Attach to lastControl to create proper vertical spacing between
widgets
+ if (lastControl != null) {
+ fdControl.top = new FormAttachment(lastControl, PropsUi.getMargin());
+ } else {
+ fdControl.top = new FormAttachment(0, PropsUi.getMargin());
+ }
}
} else {
// No label
diff --git a/ui/src/main/java/org/apache/hop/ui/core/gui/GuiToolbarWidgets.java
b/ui/src/main/java/org/apache/hop/ui/core/gui/GuiToolbarWidgets.java
index b5bab5c11a..91b4bb24f4 100644
--- a/ui/src/main/java/org/apache/hop/ui/core/gui/GuiToolbarWidgets.java
+++ b/ui/src/main/java/org/apache/hop/ui/core/gui/GuiToolbarWidgets.java
@@ -453,6 +453,10 @@ public class GuiToolbarWidgets extends BaseGuiWidgets {
SvgLabelFacade.enable(toolItem, uniqueId, imageLabel, enabled);
}
}
+ // Also update the ToolItem state for consistency
+ if (enabled != toolItem.isEnabled()) {
+ toolItem.setEnabled(enabled);
+ }
} else {
if (enabled != toolItem.isEnabled()) {
toolItem.setEnabled(enabled);
