This is an automated email from the ASF dual-hosted git repository.
ashishvijaywargiya pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
The following commit(s) were added to refs/heads/trunk by this push:
new c584f5c253 Added one more noframe file for junit test results and made
a few cosmetic changes. (#1543)
c584f5c253 is described below
commit c584f5c2535e08db7fcde012e9854e843f9f4155
Author: Ashish Vijaywargiya <[email protected]>
AuthorDate: Sun Aug 2 10:29:08 2026 +0530
Added one more noframe file for junit test results and made a few cosmetic
changes. (#1543)
Added one more noframe file for junit test results and made a few
cosmetic changes. Renamed a Type Column in Test Results. Also removed the
unused files.
---
.../ofbiz/accounting/test/FinAccountTests.java | 70 ----------------
build.gradle | 7 ++
.../ofbiz/service/test/ServiceSOAPTests.java | 71 ----------------
framework/service/testdef/servicetests.xml | 1 -
.../ofbiz/testtools/JupiterTestExtension.java | 78 ++++++++++++++++--
.../testtools/JupiterInjectionGuardsTest.java | 94 ++++++++++++++++++++++
6 files changed, 172 insertions(+), 149 deletions(-)
diff --git
a/applications/accounting/src/main/java/org/apache/ofbiz/accounting/test/FinAccountTests.java
b/applications/accounting/src/main/java/org/apache/ofbiz/accounting/test/FinAccountTests.java
deleted file mode 100644
index c24099cb0b..0000000000
---
a/applications/accounting/src/main/java/org/apache/ofbiz/accounting/test/FinAccountTests.java
+++ /dev/null
@@ -1,70 +0,0 @@
-/*
- 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.ofbiz.accounting.test;
-
-import java.math.BigDecimal;
-import java.util.HashMap;
-import java.util.Map;
-
-import org.apache.ofbiz.entity.GenericValue;
-import org.apache.ofbiz.service.ServiceUtil;
-import org.apache.ofbiz.service.testtools.OFBizTestCase;
-
-/**
- * FinAccountTests
- */
-public class FinAccountTests extends OFBizTestCase {
-
- public FinAccountTests(String name) {
- super(name);
- }
-
- /**
- * Test fin account operations.
- * @throws Exception the exception
- */
- public void testFinAccountOperations() throws Exception {
- GenericValue userLogin = getUserLogin("system");
- Map<String, Object> ctx = new HashMap<>();
- ctx.put("finAccountId", "TESTACCOUNT1");
- ctx.put("finAccountName", "Test Financial Account");
- ctx.put("finAccountTypeId", "BANK_ACCOUNT");
- ctx.put("userLogin", userLogin);
- Map<String, Object> resp = getDispatcher().runSync("createFinAccount",
ctx);
- assertTrue("Service 'createFinAccount' result success",
ServiceUtil.isSuccess(resp));
- ctx.clear();
- ctx.put("finAccountId", "TESTACCOUNT1");
- ctx.put("amount", new BigDecimal("100.00"));
- ctx.put("userLogin", userLogin);
- resp = getDispatcher().runSync("finAccountDeposit", ctx);
- assertTrue("Service 'finAccountDeposit' result success",
ServiceUtil.isSuccess(resp));
- BigDecimal balance = (BigDecimal) resp.get("balance");
- assertEquals(balance.toPlainString(), "100.00");
- ctx.clear();
- ctx.put("finAccountId", "TESTACCOUNT1");
- ctx.put("amount", new BigDecimal("50.00"));
- ctx.put("userLogin", userLogin);
- resp = getDispatcher().runSync("finAccountWithdraw", ctx);
- assertTrue("Service 'finAccountWithdraw' result success",
ServiceUtil.isSuccess(resp));
- BigDecimal previousBalance = (BigDecimal) resp.get("previousBalance");
- balance = ((BigDecimal) resp.get("balance"));
- assertEquals(balance.add(new BigDecimal("50.00")).toPlainString(),
previousBalance.toPlainString());
- }
-}
diff --git a/build.gradle b/build.gradle
index 0008973e47..d8295cf207 100644
--- a/build.gradle
+++ b/build.gradle
@@ -718,7 +718,14 @@ task createTestReports(group: sysadminGroup, description:
'Generate HTML reports
include(name: '*.xml')
}
report(format:'frames', todir:'./runtime/logs/test-results/html')
+ report(format:'noframes', todir:'./runtime/logs/test-results/html')
}
+ // Ant's stock junit-frames.xsl/junit-noframes.xsl label this column
"Type", but it actually
+ // renders the failure/error message and stack trace, not the bare
exception type - relabel
+ // it post-generation rather than vendoring and maintaining a forked
copy of Ant's templates.
+ fileTree('./runtime/logs/test-results/html') {
+ include '**/*.html'
+ }.each { file -> file.text = file.text.replace('>Type</th>', '>Failure
Reason</th>') }
}
}
diff --git
a/framework/service/src/test/java/org/apache/ofbiz/service/test/ServiceSOAPTests.java
b/framework/service/src/test/java/org/apache/ofbiz/service/test/ServiceSOAPTests.java
deleted file mode 100644
index d8085234a7..0000000000
---
a/framework/service/src/test/java/org/apache/ofbiz/service/test/ServiceSOAPTests.java
+++ /dev/null
@@ -1,71 +0,0 @@
-/*
- * 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.ofbiz.service.test;
-
-import static org.junit.jupiter.api.Assertions.assertNotNull;
-
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
-import org.apache.ofbiz.base.util.UtilDateTime;
-import org.apache.ofbiz.base.util.UtilGenerics;
-import org.apache.ofbiz.entity.GenericValue;
-import org.apache.ofbiz.testtools.JunitJupiterTest;
-import org.apache.ofbiz.testtools.JupiterTestHelper;
-import org.junit.jupiter.api.Order;
-import org.junit.jupiter.api.Test;
-
-@JunitJupiterTest
-public class ServiceSOAPTests implements JupiterTestHelper {
-
- private static final String MODULE = ServiceSOAPTests.class.getName();
-
- /**
- * Test soap simple service.
- * @throws Exception the exception
- */
- @Test
- @Order(1)
- public void testSOAPSimpleService() throws Exception {
- Map<String, Object> serviceContext = new HashMap<>();
- serviceContext.put("defaultValue", Double.valueOf("123.4567"));
- serviceContext.put("message", "Test Message !!!");
- getDispatcher().runSync("testSoapSimple", serviceContext);
- }
-
- /**
- * Test soap service.
- * @throws Exception the exception
- */
- @Test
- @Order(2)
- public void testSOAPService() throws Exception {
- Map<String, Object> serviceContext = new HashMap<>();
- GenericValue testing = getDelegator().makeValue("Testing");
- testing.put("testingId", "COMPLEX_TYPE_TEST");
- testing.put("testingTypeId", "SOAP_TEST");
- testing.put("testingName", "Complex Type Test");
- testing.put("createdStamp", UtilDateTime.nowTimestamp());
- serviceContext.put("testing", testing);
- Map<String, Object> results = getDispatcher().runSync("testSoap",
serviceContext);
- List<GenericValue> testingNodes =
UtilGenerics.cast(results.get("testingNodes"));
- assertNotNull(testingNodes);
- }
-}
diff --git a/framework/service/testdef/servicetests.xml
b/framework/service/testdef/servicetests.xml
index 9d9d636c05..93d4c63bf0 100644
--- a/framework/service/testdef/servicetests.xml
+++ b/framework/service/testdef/servicetests.xml
@@ -24,7 +24,6 @@ under the License.
<test-case case-name="service-tests"><jupiter-test-suite
class-name="org.apache.ofbiz.service.test.ServiceEngineTests"/></test-case>
<test-case case-name="service-groovy-DSL-tests"><jupiter-test-suite
class-name="org.apache.ofbiz.service.test.GroovyDslServiceEngineTests"/></test-case>
- <!-- <test-case case-name="service-soap-tests"><junit-test-suite
class-name="org.apache.ofbiz.service.test.ServiceSOAPTests"/></test-case> -->
<test-case case-name="service-entity-auto-tests"><jupiter-test-suite
class-name="org.apache.ofbiz.service.test.ServiceEntityAutoTests"/></test-case>
<test-case case-name="load-service-test-data">
diff --git
a/framework/testtools/src/main/java/org/apache/ofbiz/testtools/JupiterTestExtension.java
b/framework/testtools/src/main/java/org/apache/ofbiz/testtools/JupiterTestExtension.java
index b58fcebdec..b881cc4987 100644
---
a/framework/testtools/src/main/java/org/apache/ofbiz/testtools/JupiterTestExtension.java
+++
b/framework/testtools/src/main/java/org/apache/ofbiz/testtools/JupiterTestExtension.java
@@ -19,6 +19,7 @@
package org.apache.ofbiz.testtools;
import java.lang.reflect.Field;
+import java.lang.reflect.Method;
import java.util.HashMap;
import java.util.Map;
import java.util.regex.Matcher;
@@ -27,6 +28,7 @@ import java.util.regex.Pattern;
import org.apache.ofbiz.base.util.Debug;
import org.apache.ofbiz.entity.Delegator;
import org.apache.ofbiz.service.LocalDispatcher;
+import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.extension.ConditionEvaluationResult;
import org.junit.jupiter.api.extension.ExecutionCondition;
import org.junit.jupiter.api.extension.ExtensionContext;
@@ -35,6 +37,7 @@ import
org.junit.jupiter.api.extension.ParameterResolutionException;
import org.junit.jupiter.api.extension.ParameterResolver;
import org.junit.jupiter.api.extension.TestInstancePostProcessor;
import org.junit.platform.engine.TestExecutionResult;
+import org.junit.platform.engine.support.descriptor.MethodSource;
import org.junit.platform.launcher.Launcher;
import org.junit.platform.launcher.LauncherDiscoveryRequest;
import org.junit.platform.launcher.TestExecutionListener;
@@ -283,7 +286,43 @@ public class JupiterTestExtension implements
ParameterResolver, TestInstancePost
"org.junit.jupiter.api.MethodOrderer$OrderAnnotation")
.configurationParameter("junit.jupiter.execution.parallel.enabled", "false")
.build();
- this.testCaseCount = (int)
launcher.discover(request).countTestIdentifiers(TestIdentifier::isTest);
+ this.testCaseCount = (int)
launcher.discover(request).countTestIdentifiers(
+ id -> id.isTest() && !isStaticallyDisabled(id));
+ }
+
+ /**
+ * Excludes {@literal @}Disabled methods/classes from countTestCases()
so it agrees with the
+ * TestResult.runCount() TestRunContainer actually logs: run() below
never calls startTest()
+ * for a disabled test (see executionSkipped()), so runCount() never
counts it either. Discovery
+ * alone can't see every reason a test might not run - an
ExecutionCondition like this class's own
+ * evaluateExecutionCondition() is only evaluated at execution time -
but a bare {@literal @}Disabled
+ * is visible right here via reflection on the MethodSource, which
covers the common case cheaply.
+ * Any resolution failure falls back to "not disabled" (matches the
old, over-counting behavior)
+ * rather than risk hiding a test that actually runs.
+ */
+ private static boolean isStaticallyDisabled(TestIdentifier identifier)
{
+ return identifier.getSource()
+ .filter(MethodSource.class::isInstance)
+ .map(MethodSource.class::cast)
+ .map(JupiterTestSuite::isDisabledMethodSource)
+ .orElse(false);
+ }
+
+ private static boolean isDisabledMethodSource(MethodSource source) {
+ try {
+ Class<?> testClass = Class.forName(source.getClassName());
+ if (testClass.isAnnotationPresent(Disabled.class)) {
+ return true;
+ }
+ for (Method method : testClass.getDeclaredMethods()) {
+ if (method.getName().equals(source.getMethodName()) &&
method.isAnnotationPresent(Disabled.class)) {
+ return true;
+ }
+ }
+ } catch (ClassNotFoundException e) {
+ return false;
+ }
+ return false;
}
void setDelegator(Delegator delegator) {
@@ -309,7 +348,7 @@ public class JupiterTestExtension implements
ParameterResolver, TestInstancePost
@Override
public void executionStarted(TestIdentifier
testIdentifier) {
if (testIdentifier.isTest()) {
- Test leaf = new
JupiterLeafTest(reportingName(testIdentifier), testClass.getName());
+ Test leaf = new
JupiterLeafTest(reportingName(testIdentifier, testClass), testClass.getName());
leafTests.put(testIdentifier.getUniqueId(), leaf);
result.startTest(leaf);
}
@@ -329,6 +368,17 @@ public class JupiterTestExtension implements
ParameterResolver, TestInstancePost
return;
}
Test leaf =
leafTests.get(testIdentifier.getUniqueId());
+ if (testExecutionResult.getStatus() ==
TestExecutionResult.Status.ABORTED) {
+ // A JUnit 5 Assumptions.assumeTrue/assumeFalse
failure: a deliberate skip, not a
+ // defect, so it is reported the same way
executionSkipped() reports a @Disabled
+ // test - logged, not routed through
addFailure()/addError() - even though, unlike
+ // a @Disabled test, startTest() already ran for
it and endTest() still must too.
+
testExecutionResult.getThrowable().ifPresent(throwable ->
+ Debug.logInfo("[JUNIT] ABORTED: " +
testIdentifier.getDisplayName()
+ + " (" + testClass.getName() + ")
- " + throwable.getMessage(), MODULE));
+ result.endTest(leaf);
+ return;
+ }
testExecutionResult.getThrowable().ifPresent(throwable
-> {
if (throwable instanceof AssertionError) {
result.addFailure(leaf, new
AssertionFailedError(throwable.getMessage()));
@@ -356,15 +406,29 @@ public class JupiterTestExtension implements
ParameterResolver, TestInstancePost
* is replaced with the test's own @ParameterizedTest(name=...)
display text (e.g. "[1] exampleTypeId=CONTRIVED"
* becomes "shouldCreateExampleAcrossTypes[exampleTypeId=CONTRIVED]"),
so each row is identifiable
* without needing to click into it.
+ *
+ * <p>Prefixed with the test class's simple name
("AutoAcctgAdminTests.testXxx") because that class
+ * is otherwise invisible in the JUnit XML/HTML report: every
Jupiter-sourced {@code <testcase>} in a
+ * suite shares one {@code classname}, {@code JupiterLeafTest}'s own
class
+ * ({@code
org.apache.ofbiz.testtools.JupiterTestExtension$JupiterTestSuite$JupiterLeafTest}),
since
+ * Ant's {@code JUnitVersionHelper.getTestCaseClassName()} derives
{@code classname} from
+ * {@code test.getClass().getName()} with no hook to override it - the
only exception is a test
+ * object that literally is {@code
junit.framework.JUnit4TestCaseFacade}, whose package-private
+ * constructor rules out subclassing it from this package. Two
different Jupiter classes bundled into
+ * the same {@code <test-suite>} can therefore define same-named
methods (a real collision:
+ * {@code AutoAcctgAdminTests} and {@code AutoAcctgAgreementTests}
both have a
+ * {@code testAddPaymentMethodTypeGlAssignment}) and be
indistinguishable in the report without this
+ * prefix, since {@code classname} can't carry it and bare {@code
name} previously didn't either.
*/
- private static String reportingName(TestIdentifier testIdentifier) {
+ private static String reportingName(TestIdentifier testIdentifier,
Class<?> testClass) {
String withoutParamTypes =
testIdentifier.getLegacyReportingName().replaceAll("\\([^)]*\\)", "");
Matcher indexSuffix = INDEX_SUFFIX.matcher(withoutParamTypes);
- if (!indexSuffix.matches()) {
- return withoutParamTypes;
+ String bareName = withoutParamTypes;
+ if (indexSuffix.matches()) {
+ String invocationLabel =
testIdentifier.getDisplayName().replaceFirst("^\\[\\d+]\\s*", "");
+ bareName = indexSuffix.group(1) + "[" + invocationLabel + "]";
}
- String invocationLabel =
testIdentifier.getDisplayName().replaceFirst("^\\[\\d+]\\s*", "");
- return indexSuffix.group(1) + "[" + invocationLabel + "]";
+ return testClass.getSimpleName() + "." + bareName;
}
/**
diff --git
a/framework/testtools/src/test/java/org/apache/ofbiz/testtools/JupiterInjectionGuardsTest.java
b/framework/testtools/src/test/java/org/apache/ofbiz/testtools/JupiterInjectionGuardsTest.java
index 77bdc9a82e..4d7b013f1c 100644
---
a/framework/testtools/src/test/java/org/apache/ofbiz/testtools/JupiterInjectionGuardsTest.java
+++
b/framework/testtools/src/test/java/org/apache/ofbiz/testtools/JupiterInjectionGuardsTest.java
@@ -27,8 +27,12 @@ import java.util.List;
import org.apache.ofbiz.entity.Delegator;
import org.apache.ofbiz.service.LocalDispatcher;
import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assumptions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.api.extension.ParameterContext;
import org.junit.jupiter.api.extension.ParameterResolutionException;
@@ -39,6 +43,7 @@ import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.everyItem;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.sameInstance;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -155,6 +160,48 @@ class JupiterInjectionGuardsTest {
}
}
+ @Test
+ void assumptionSkipIsReportedAsInfoNotAsAnError() {
+ Delegator delegator = mock(Delegator.class);
+ LocalDispatcher dispatcher = mock(LocalDispatcher.class);
+
+ JupiterTestExtension.JupiterTestSuite suite =
+ new
JupiterTestExtension.JupiterTestSuite(AssumptionFixture.class);
+ suite.setDelegator(delegator);
+ suite.setDispatcher(dispatcher);
+ TestResult result = new TestResult();
+ suite.run(result);
+
+ // startTest()/endTest() still ran (unlike a @Disabled test, which
never starts), so the
+ // test counts toward runCount(); it must not also land in
errorCount()/failureCount() the
+ // way a bare Throwable used to before ABORTED got its own branch in
executionFinished().
+ assertThat(result.runCount(), is(1));
+ assertThat(result.errorCount(), is(0));
+ assertThat(result.failureCount(), is(0));
+ }
+
+ @Test
+ void countTestCasesExcludesDisabledMethods() {
+ JupiterTestExtension.JupiterTestSuite suite =
+ new
JupiterTestExtension.JupiterTestSuite(DisabledCountFixture.class);
+
+ assertThat(suite.countTestCases(), is(1));
+ }
+
+ @Test
+ void beforeAllStaticMethodReceivesDelegatorViaParameterResolution() {
+ Delegator delegator = mock(Delegator.class);
+ LocalDispatcher dispatcher = mock(LocalDispatcher.class);
+
+ JupiterTestExtension.JupiterTestSuite suite =
+ new
JupiterTestExtension.JupiterTestSuite(StaticInjectionFixture.class);
+ suite.setDelegator(delegator);
+ suite.setDispatcher(dispatcher);
+ suite.run(new TestResult());
+
+ assertThat(StaticInjectionFixture.capturedDelegator(),
sameInstance(delegator));
+ }
+
//ALLOW PUBLIC FIELDS
static class CorrectlyNamedFields {
Delegator delegator;
@@ -209,4 +256,51 @@ class JupiterInjectionGuardsTest {
void dummy(Delegator delegator) {
}
}
+
+ @Tag(JupiterTestExtension.INTEGRATION_TAG)
+ @ExtendWith(JupiterTestExtension.class)
+ static class AssumptionFixture {
+ @Test
+ void skipsViaAssumption() {
+ Assumptions.assumeTrue(false, "deliberately never true - exists
only to abort this test");
+ }
+ }
+
+ // No @ExtendWith needed: countTestCases() is read straight off the
constructor's discovery
+ // result, before run() would ever need
CURRENT_DELEGATOR/CURRENT_DISPATCHER armed.
+ @Tag(JupiterTestExtension.INTEGRATION_TAG)
+ static class DisabledCountFixture {
+ @Test
+ void enabledTest() {
+ }
+
+ @Disabled("only used to verify countTestCases() excludes a
statically-disabled method")
+ @Test
+ void disabledTest() {
+ }
+ }
+
+ // Unlike ThreadRecordingFixture above, this one needs
@ExtendWith(JupiterTestExtension.class)
+ // active: a static @BeforeAll method has no test instance for
postProcessTestInstance() to
+ // inject a field into, so resolveParameter() - reached only through the
registered extension -
+ // is the only injection path the javadoc documents for it, and this is
what exercises that path.
+ @Tag(JupiterTestExtension.INTEGRATION_TAG)
+ @ExtendWith(JupiterTestExtension.class)
+ static class StaticInjectionFixture {
+ private static Delegator capturedDelegator;
+
+ @BeforeAll
+ static void captureDelegator(Delegator delegator) {
+ capturedDelegator = delegator;
+ }
+
+ static Delegator capturedDelegator() {
+ return capturedDelegator;
+ }
+
+ @Test
+ void triggersBeforeAll() {
+ // No-op: exists only so the class has a @Test method for
@BeforeAll to run ahead of.
+ }
+ }
}
