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 47d38419fc [CI] fix floating test numbers (#8547)
47d38419fc is described below
commit 47d38419fccdcfc23625248608c95db23f1e816a
Author: Hans Van Akelyen <[email protected]>
AuthorDate: Wed Sep 23 11:34:22 2026 +0200
[CI] fix floating test numbers (#8547)
GuiRegistry can have the same items multiple times because of
environment.init() this causes our ci numbers to float around
---
.../apache/hop/core/gui/plugin/GuiRegistry.java | 16 ++++-
.../hop/core/gui/plugin/GuiRegistryTest.java | 78 ++++++++++++++++++++++
.../java/org/apache/hop/beam/gui/WelcomeBeam.java | 2 +-
.../hop/ui/testing/DisabledGuiWidgetsTestBase.java | 36 ++++++++++
4 files changed, 129 insertions(+), 3 deletions(-)
diff --git a/core/src/main/java/org/apache/hop/core/gui/plugin/GuiRegistry.java
b/core/src/main/java/org/apache/hop/core/gui/plugin/GuiRegistry.java
index 22b3f5504c..0c982ae6db 100644
--- a/core/src/main/java/org/apache/hop/core/gui/plugin/GuiRegistry.java
+++ b/core/src/main/java/org/apache/hop/core/gui/plugin/GuiRegistry.java
@@ -372,7 +372,7 @@ public class GuiRegistry {
// See if we need to disable something of if something is disabled
already...
// In those scenarios we ignore the GuiWidgetElement
//
- GuiElements existing = guiElements.findChild(guiElement.id());
+ GuiElements existing = guiElements.findChild(child.getId());
if (existing != null && existing.isIgnored()) {
return;
}
@@ -380,6 +380,12 @@ public class GuiRegistry {
existing.setIgnored(true);
return;
}
+ // Already registered: HopGuiEnvironment.init() can run more than once in
the same JVM, and a
+ // second copy of the element would be built as a second widget with the
same id.
+ //
+ if (existing != null) {
+ return;
+ }
guiElements.getChildren().add(child);
}
@@ -435,7 +441,7 @@ public class GuiRegistry {
// See if we need to disable something of if something is disabled
already...
// In those scenarios we ignore the GuiWidgetElement
//
- GuiElements existing = guiElements.findChild(guiElement.id());
+ GuiElements existing = guiElements.findChild(child.getId());
if (existing != null && existing.isIgnored()) {
return;
}
@@ -443,6 +449,12 @@ public class GuiRegistry {
existing.setIgnored(true);
return;
}
+ // Already registered: HopGuiEnvironment.init() can run more than once in
the same JVM, and a
+ // second copy of the element would be built as a second widget with the
same id.
+ //
+ if (existing != null) {
+ return;
+ }
guiElements.getChildren().add(child);
}
diff --git
a/core/src/test/java/org/apache/hop/core/gui/plugin/GuiRegistryTest.java
b/core/src/test/java/org/apache/hop/core/gui/plugin/GuiRegistryTest.java
index 2275716b08..eb732a7809 100644
--- a/core/src/test/java/org/apache/hop/core/gui/plugin/GuiRegistryTest.java
+++ b/core/src/test/java/org/apache/hop/core/gui/plugin/GuiRegistryTest.java
@@ -19,7 +19,10 @@ package org.apache.hop.core.gui.plugin;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -53,4 +56,79 @@ class GuiRegistryTest {
Object verifyObject1111 = registry.findGuiPluginObject("hop-gui-id1",
"class1", "instance1");
assertEquals(object1, verifyObject1111);
}
+
+ @Test
+ void registeringAFieldWidgetTwiceKeepsOneElement() throws Exception {
+ String dataClassName = getClass().getName() + "#fieldTwice";
+ Field field = WidgetSample.class.getDeclaredField("name");
+ GuiWidgetElement element = field.getAnnotation(GuiWidgetElement.class);
+
+ registry.addGuiWidgetElement(dataClassName, element, field);
+ registry.addGuiWidgetElement(dataClassName, element, field);
+
+ GuiElements elements = registry.findGuiElements(dataClassName,
WidgetSample.PARENT_ID);
+ assertEquals(1, elements.getChildren().size());
+ }
+
+ @Test
+ void registeringAWidgetWithoutAnExplicitIdTwiceKeepsOneElement() throws
Exception {
+ // Without an id in the annotation the element takes the field name as its
id.
+ String dataClassName = getClass().getName() + "#noIdTwice";
+ Field field = WidgetSample.class.getDeclaredField("sampleSize");
+ GuiWidgetElement element = field.getAnnotation(GuiWidgetElement.class);
+
+ registry.addGuiWidgetElement(dataClassName, element, field);
+ registry.addGuiWidgetElement(dataClassName, element, field);
+
+ GuiElements elements = registry.findGuiElements(dataClassName,
WidgetSample.PARENT_ID);
+ assertEquals(1, elements.getChildren().size());
+ assertEquals("sampleSize", elements.getChildren().get(0).getId());
+ }
+
+ @Test
+ void registeringAMethodWidgetTwiceKeepsOneElement() throws Exception {
+ String dataClassName = getClass().getName() + "#methodTwice";
+ Method method = WidgetSample.class.getDeclaredMethod("browse");
+ GuiWidgetElement element = method.getAnnotation(GuiWidgetElement.class);
+ ClassLoader classLoader = getClass().getClassLoader();
+
+ registry.addGuiWidgetElement(element, method, dataClassName, classLoader);
+ registry.addGuiWidgetElement(element, method, dataClassName, classLoader);
+
+ GuiElements elements = registry.findGuiElements(dataClassName,
WidgetSample.PARENT_ID);
+ assertEquals(1, elements.getChildren().size());
+ }
+
+ @Test
+ void anIgnoredDeclarationStillHidesARegisteredWidget() throws Exception {
+ String dataClassName = getClass().getName() + "#ignored";
+ Field field = WidgetSample.class.getDeclaredField("name");
+ Field ignoredField = WidgetSample.class.getDeclaredField("hiddenName");
+
+ registry.addGuiWidgetElement(dataClassName,
field.getAnnotation(GuiWidgetElement.class), field);
+ registry.addGuiWidgetElement(
+ dataClassName, ignoredField.getAnnotation(GuiWidgetElement.class),
ignoredField);
+
+ GuiElements elements = registry.findGuiElements(dataClassName,
WidgetSample.PARENT_ID);
+ assertEquals(1, elements.getChildren().size());
+ assertTrue(elements.getChildren().get(0).isIgnored());
+ }
+
+ private static class WidgetSample {
+ static final String PARENT_ID = "GuiRegistryTest-parent";
+
+ @GuiWidgetElement(id = "name", type = GuiElementType.TEXT, parentId =
PARENT_ID)
+ private String name;
+
+ @GuiWidgetElement(id = "name", type = GuiElementType.TEXT, parentId =
PARENT_ID, ignored = true)
+ private String hiddenName;
+
+ @GuiWidgetElement(type = GuiElementType.TEXT, parentId = PARENT_ID)
+ private String sampleSize;
+
+ @GuiWidgetElement(id = "browse", type = GuiElementType.BUTTON, parentId =
PARENT_ID)
+ void browse() {
+ // Only the annotation matters here.
+ }
+ }
}
diff --git
a/plugins/engines/beam/src/main/java/org/apache/hop/beam/gui/WelcomeBeam.java
b/plugins/engines/beam/src/main/java/org/apache/hop/beam/gui/WelcomeBeam.java
index 502a0be4e8..1113baf292 100644
---
a/plugins/engines/beam/src/main/java/org/apache/hop/beam/gui/WelcomeBeam.java
+++
b/plugins/engines/beam/src/main/java/org/apache/hop/beam/gui/WelcomeBeam.java
@@ -109,7 +109,7 @@ public class WelcomeBeam {
private static final String EXAMPLE2_FILE =
"${PROJECT_HOME}/beam/pipelines/complex.hpl";
@GuiWidgetElement(
- id = "WelcomeBeam.11000.example1",
+ id = "WelcomeBeam.11010.example2",
parentId = WELCOME_BEAM_PARENT_ID,
type = GuiElementType.LINK,
label =
diff --git
a/ui/src/test/java/org/apache/hop/ui/testing/DisabledGuiWidgetsTestBase.java
b/ui/src/test/java/org/apache/hop/ui/testing/DisabledGuiWidgetsTestBase.java
index 3ec7406d55..77d19b48d0 100644
--- a/ui/src/test/java/org/apache/hop/ui/testing/DisabledGuiWidgetsTestBase.java
+++ b/ui/src/test/java/org/apache/hop/ui/testing/DisabledGuiWidgetsTestBase.java
@@ -18,6 +18,7 @@
package org.apache.hop.ui.testing;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.DynamicTest.dynamicTest;
import java.lang.reflect.Method;
@@ -26,6 +27,7 @@ import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.apache.hop.core.gui.plugin.GuiElements;
import org.apache.hop.core.gui.plugin.GuiRegistry;
@@ -123,6 +125,40 @@ public abstract class DisabledGuiWidgetsTestBase extends
SwtBotTestBase {
+ ", so the tests below would pass without testing anything");
}
+ /**
+ * Every UI test class in a module shares one JVM and one registry, and
several of them call
+ * {@link HopGuiEnvironment#init()}. A registry that appends a second copy
of each widget on every
+ * init makes the composite cases below grow with the number of classes that
happened to run
+ * first, and a composite built from it holds two widgets with the same id.
Registering once more
+ * here makes that visible no matter in which order the classes run.
+ */
+ @Test
+ void registeringTheGuiPluginsAgainDoesNotDuplicateWidgets() throws Exception
{
+ HopGuiEnvironment.init();
+
+ Map<Element, Long> occurrences =
+ compositeElements().stream()
+ .collect(Collectors.groupingBy(element -> element,
Collectors.counting()));
+ List<String> duplicates =
+ occurrences.entrySet().stream()
+ .filter(entry -> entry.getValue() > 1)
+ .map(
+ entry ->
+ entry.getKey().owner()
+ + " "
+ + entry.getKey().container()
+ + " '"
+ + entry.getKey().id()
+ + "' x"
+ + entry.getValue())
+ .sorted()
+ .toList();
+
+ assertTrue(
+ duplicates.isEmpty(),
+ "Widgets registered more than once in the GuiRegistry: " + duplicates);
+ }
+
@TestFactory
Stream<DynamicTest> disablingAWidgetMustNotBreakItsComposite() {
return casesFor(compositeElements(), "composite", this::buildComposite);
