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 a5cc17f21f Add generic, component-agnostic test-run REST endpoint
(#1699)
a5cc17f21f is described below
commit a5cc17f21fbd76764c945f0194ea4dbda3055355
Author: Ashish Vijaywargiya <[email protected]>
AuthorDate: Sat Aug 22 17:19:08 2026 +0530
Add generic, component-agnostic test-run REST endpoint (#1699)
Title: Add generic, component-agnostic test-run REST endpoint
Adds a single REST endpoint, in framework/testtools, that can trigger and
poll a testdef
test-suite run for any component:
POST /rest/testtools/testruns/{componentName}
GET /rest/testtools/testruns/{runId}
componentName is supplied as a URL path parameter and bound into the
service context by the
REST framework's existing path-parameter mechanism - the same mechanism the
GET operation's
{runId} already used. The endpoint wires directly to the
runTestSuite/getTestRunStatus services
that already existed, unscoped, in
framework/testtools/servicedef/services.xml.
Why
This replaces a per-component pattern (see the companion ofbiz-plugins pull
request (apache/ofbiz-plugins#374)) that
required copy-pasting a groovy wrapper class, two duplicated <service>
definitions, and a
component-owned *.rest.xml into every adopting component, purely to
hard-code that one
component's name. The generic endpoint needs zero new files for any current
or future component
that already has a real test-suite.
TestRunServices.runScopedTestSuite/getScopedTestRunStatus - the helper
methods that existed only
to support the old per-component wrapper - are removed, along with their
tests.
Security fix included
The endpoint's componentName normally comes from the URL path, but REST
attribute binding merges
body/path/query/header values onto the same context map, so a caller could
previously send an
empty componentName (e.g. an empty query parameter) and bypass the
per-component
test.api.enabled.<componentName> toggle entirely, falling back to an
unscoped sweep across every
component's tests. runTestSuite now rejects a blank componentName outright,
restoring the
fail-closed behavior the old per-component wrapper always had.
Testing
- Unit tests for TestRunServices: 9/9 passing.
- Manual verification against a running server: triggered several
components' suites through the
new endpoint, confirmed PASSED status with the correct componentName
reported; confirmed a
deliberately wrong testParams override produces the expected different
failure, proving the
override reaches the underlying assertion; confirmed the old
per-component URL naming pattern
is not a real route.
- Full testIntegration run across the whole codebase: 658/658 tests
passing, zero regressions.
Dependency on the companion pull request
This should merge together with, or after, the companion ofbiz-plugins pull
request (apache/ofbiz-plugins#374). That PR
deletes the wrapper scripts that call
runScopedTestSuite/getScopedTestRunStatus - merging this
PR first while those scripts are still present would leave them calling
methods that no longer
exist.
---
framework/testtools/api/testruns.rest.xml | 45 ++++
framework/testtools/servicedef/services.xml | 24 +--
.../apache/ofbiz/testtools/TestRunServices.java | 127 +++++------
.../ofbiz/testtools/TestRunServicesTest.java | 232 +++++----------------
4 files changed, 152 insertions(+), 276 deletions(-)
diff --git a/framework/testtools/api/testruns.rest.xml
b/framework/testtools/api/testruns.rest.xml
new file mode 100644
index 0000000000..be2ed9e30b
--- /dev/null
+++ b/framework/testtools/api/testruns.rest.xml
@@ -0,0 +1,45 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+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.
+-->
+
+<api name="TestToolsApi"
+ displayName="Test Tools REST API"
+ path="testtools"
+ description="Generic, component-agnostic REST endpoint to trigger and
poll testdef test-suite
+ runs for any component, identified by a componentName URL path
parameter."
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+
xsi:noNamespaceSchemaLocation="https://ofbiz.apache.org/dtds/rest-api.xsd";>
+
+ <resource name="TestRunResource"
+ path="testruns"
+ displayName="Test Run"
+ description="Trigger and poll a testdef test-suite run for any
component."
+ primaryPermission="TESTEXEC"
+ mainAction="ADMIN">
+
+ <operation verb="post" description="Trigger a test suite run for the
given component"
+ path="{componentName}" consumes="application/json">
+ <service name="runTestSuite"/>
+ </operation>
+
+ <operation verb="get" description="Get a test run's status"
path="{runId}">
+ <service name="getTestRunStatus"/>
+ </operation>
+ </resource>
+</api>
diff --git a/framework/testtools/servicedef/services.xml
b/framework/testtools/servicedef/services.xml
index 626ff5cce4..3435c2c0cb 100644
--- a/framework/testtools/servicedef/services.xml
+++ b/framework/testtools/servicedef/services.xml
@@ -53,13 +53,14 @@ under the License.
location="org.apache.ofbiz.testtools.TestRunServices"
invoke="runTestSuite">
<description>Kicks off a testdef test-suite run asynchronously via the
in-JVM Jupiter test
engine and returns a runId immediately; poll getTestRunStatus for
progress/results.
- Gated by the test.api.enabled config flag and the TESTEXEC_ADMIN
permission. When
- componentName is set, the per-component
test.api.enabled.<componentName>
- override (SystemProperty-overridable, defaults to enabled) is also
consulted. Not
- intended for direct *.rest.xml exposure - a component-branded REST
endpoint must wrap
- this with a component-scoped service (see
TestRunServices.runScopedTestSuite and
- plugins/example's ExampleTestRunServices for the pattern), or the
endpoint can trigger
- any component's tests. testMethodName optionally scopes the run to
one
+ Gated by the test.api.enabled config flag and the TESTEXEC_ADMIN
permission. componentName
+ is required (rejected if blank) - the per-component
test.api.enabled.<componentName>
+ override (SystemProperty-overridable, defaults to enabled) is
always consulted as a result.
+ Exposed directly via the generic, framework-owned
framework/testtools/api/testruns.rest.xml
+ endpoint (POST /rest/testtools/testruns/{componentName}) - a
component's own *.rest.xml must
+ never expose this service directly, since that would make an
intentionally generic,
+ multi-component service masquerade as scoped to just that one
component's branded URL.
+ testMethodName optionally scopes the run to one
@Test/@ParameterizedTest method within the class testCaseName
resolves to - requires
testCaseName, and only applies when it resolves to a
jupiter-test-suite.</description>
<attribute name="componentName" type="String" mode="IN"
optional="true"/>
@@ -73,11 +74,10 @@ under the License.
<service name="getTestRunStatus" engine="java" auth="true"
location="org.apache.ofbiz.testtools.TestRunServices"
invoke="getTestRunStatus">
<description>Reads a runTestSuite-triggered run's current status
(QUEUED/RUNNING/PASSED/
- FAILED/ERROR) and result summary from the in-memory
TestRunTracker. Not intended for
- direct *.rest.xml exposure - a component-branded REST endpoint
must wrap this with a
- component-scoped service (see
TestRunServices.getScopedTestRunStatus and
- plugins/example's ExampleTestRunServices for the pattern), or the
endpoint can poll any
- component's tests.</description>
+ FAILED/ERROR) and result summary from the in-memory TestRunTracker.
+ Exposed directly via the generic, framework-owned
framework/testtools/api/testruns.rest.xml
+ endpoint (GET /rest/testtools/testruns/{runId}) - same caution as
runTestSuite above applies to
+ any component-owned *.rest.xml.</description>
<attribute name="runId" type="String" mode="IN" optional="false"/>
<attribute name="status" type="String" mode="OUT" optional="true"/>
<attribute name="componentName" type="String" mode="OUT"
optional="true"/>
diff --git
a/framework/testtools/src/main/java/org/apache/ofbiz/testtools/TestRunServices.java
b/framework/testtools/src/main/java/org/apache/ofbiz/testtools/TestRunServices.java
index 15ea1334fc..d0d48f6cec 100644
---
a/framework/testtools/src/main/java/org/apache/ofbiz/testtools/TestRunServices.java
+++
b/framework/testtools/src/main/java/org/apache/ofbiz/testtools/TestRunServices.java
@@ -21,7 +21,6 @@ package org.apache.ofbiz.testtools;
import java.io.File;
import java.io.FileOutputStream;
import java.util.Collections;
-import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
@@ -101,19 +100,32 @@ import org.apache.ofbiz.testtools.report.TestRunManifest;
* independently) is not undone. Anyone deciding whether to enable {@code
test.api.enabled} (see
* testtools.properties) should weigh both of the limitations above.
*
- * <p><b>Do not expose {@link #runTestSuite}/{@link #getTestRunStatus}
directly in a component's own
- * {@code *.rest.xml}.</b> Both accept/report an arbitrary {@code
componentName} and so can trigger or
- * poll any component's tests, not just the exposing component's own - a
component-branded REST
- * endpoint must instead wrap {@link #runScopedTestSuite}/{@link
#getScopedTestRunStatus} with its own
- * fixed component name, the way {@code plugins/example}'s {@code
ExampleTestRunServices} does. Writing
- * {@code <service name="runTestSuite"/>} straight into a {@code *.rest.xml}
reproduces the exact
- * cross-component-reach problem the scoped wrappers exist to close.
+ * <p><b>{@link #runTestSuite}/{@link #getTestRunStatus} are exposed
directly</b>, via the single,
+ * generic, framework-owned {@code framework/testtools/api/testruns.rest.xml}
endpoint
+ * ({@code POST /rest/testtools/testruns/{componentName}}, {@code GET
/rest/testtools/testruns/{runId}}).
+ * {@code componentName} normally comes from that URL's path parameter, but
REST attribute binding
+ * merges body/path/query/header sources onto the same context map, so a
caller can still send a
+ * different value (e.g. a same-named query parameter) or an empty one.
Neither is a vulnerability
+ * this endpoint needs to close: it is deliberately unscoped, so resolving a
*different, real*
+ * componentName than the URL implies is exactly what it's for, not a bypass
of anything. An *empty*
+ * componentName is the one case {@link #runTestSuite} does reject outright
(see the check near the
+ * top of that method) - left unchecked, it would silently degrade into an
unscoped sweep across every
+ * component's tests, bypassing the per-component {@code
test.api.enabled.<componentName>} gate below
+ * entirely. <b>Do not</b> write {@code <service name="runTestSuite"/>} (or
{@code getTestRunStatus})
+ * into a *component's own* {@code *.rest.xml}: doing so would make an
intentionally generic service
+ * masquerade as scoped to just that component's branded URL, which is exactly
the confusion this
+ * class's single generic endpoint exists to avoid.
*/
public final class TestRunServices {
private static final String MODULE = TestRunServices.class.getName();
private static final String RESOURCE = "testtools";
private static final String TESTEXEC_PERMISSION = "TESTEXEC_ADMIN";
+ // Client-facing text for both disabled-API rejections below deliberately
omits the config
+ // property name/value (e.g. "test.api.enabled.example=false") - that
detail would let a REST
+ // caller enumerate/guess the per-component toggle naming convention. The
full detail is still
+ // captured server-side via the Debug.logWarning calls at each rejection
site.
+ private static final String API_DISABLED_MESSAGE = "The test execution API
is disabled in this environment.";
static final TestRunTracker TRACKER = new TestRunTracker();
private static final ExecutorService EXECUTOR =
Executors.newSingleThreadExecutor(runnable -> {
@@ -152,24 +164,38 @@ public final class TestRunServices {
if (!apiEnabled) {
Debug.logWarning("runTestSuite: rejected for user '" + userLoginId
+ "', suite '" + suiteName + "'"
+ " - test.api.enabled is false", MODULE);
- return ServiceUtil.returnError("The test execution API is disabled
in this environment (test.api.enabled=false)");
+ return ServiceUtil.returnError(API_DISABLED_MESSAGE);
+ }
+
+ // componentName is required, not merely conventional: the only REST
route to this service is
+ // the generic framework/testtools/api/testruns.rest.xml endpoint,
which supplies it as a URL
+ // path parameter - but REST attribute binding still lets a caller
override that with an empty
+ // value (e.g. a same-named query parameter). Failing closed here, not
open: skipping this
+ // check would let componentName reach
ComponentConfig.matchingComponentName as null/blank,
+ // which matches every component - silently turning a request into an
unscoped sweep across
+ // every component's tests and bypassing the per-component
test.api.enabled.<componentName>
+ // gate immediately below entirely. See this class's javadoc for the
broader
+ // caller-suppliable-componentName discussion this guard is part of.
+ if (UtilValidate.isEmpty(componentName)) {
+ Debug.logWarning("runTestSuite: rejected for user '" + userLoginId
+ "', suite '" + suiteName + "'"
+ + " - componentName is required", MODULE);
+ return ServiceUtil.returnError("runTestSuite requires a
componentName");
}
// Per-component override of the global flag above: lets one
component's REST-triggered test
// run be disabled (or re-enabled) live via a SystemProperty row,
without touching every other
// component's access. Defaults to enabled ("true") when unset, so a
component that never sets
- // this behaves exactly as it did before this check existed. Skipped
when componentName is
- // blank - an unscoped multi-component suite-name lookup isn't
attributable to one component's
- // flag. See
plugins/supporting-docs/specs/2026-08-21-per-component-test-api-toggle-design.md.
- if (UtilValidate.isNotEmpty(componentName)) {
- boolean componentEnabled = "true".equalsIgnoreCase(
- readStringProperty(dctx.getDelegator(),
"test.api.enabled." + componentName, "true"));
- if (!componentEnabled) {
- Debug.logWarning("runTestSuite: rejected for user '" +
userLoginId + "', suite '" + suiteName + "'"
- + " - test.api.enabled." + componentName + " is
false", MODULE);
- return ServiceUtil.returnError("The test execution API is
disabled for component '" + componentName
- + "' in this environment (test.api.enabled." +
componentName + "=false)");
- }
+ // this behaves exactly as it did before this check existed.
componentName is guaranteed
+ // non-blank by the guard above, so this always runs now - there is no
longer an unscoped,
+ // no-componentName path through this method to skip it for.
+ // See
plugins/supporting-docs/specs/2026-08-21-per-component-test-api-toggle-design.md.
+ boolean componentEnabled = "true".equalsIgnoreCase(
+ readStringProperty(dctx.getDelegator(), "test.api.enabled." +
componentName, "true"));
+ if (!componentEnabled) {
+ Debug.logWarning("runTestSuite: rejected for user '" + userLoginId
+ "', suite '" + suiteName + "'"
+ + " - test.api.enabled." + componentName + " is false",
MODULE);
+ return ServiceUtil.returnError("The test execution API is disabled
for component '" + componentName
+ + "' in this environment.");
}
// testMethodName reuses the exact same fail-closed validators the
ofbiz --test method=
@@ -282,32 +308,6 @@ public final class TestRunServices {
return result;
}
- /**
- * Runs {@link #runTestSuite} with {@code componentName} forced to {@code
fixedComponentName},
- * regardless of whatever value (if any) the caller's own context map
contains - a caller-supplied
- * componentName is silently overwritten, never honored. Built for
component-scoped wrapper
- * services (e.g. plugins/example's runExampleTestSuite): builds a new
context map rather than
- * mutating the one it's given, the same defensive-copy discipline
runTestSuite itself already
- * applies to testParams - a caller-controlled map must never be assumed
safe to mutate in place.
- * @param dctx the dispatch context
- * @param context the caller's service context - not mutated
- * @param fixedComponentName the only component this call is allowed to
resolve suites from
- * @return the runTestSuite result
- */
- public static Map<String, Object> runScopedTestSuite(DispatchContext dctx,
Map<String, ?> context,
- String fixedComponentName) {
- // Fail closed, not open: an empty/null fixedComponentName must never
reach runTestSuite's
- // context map. ComponentConfig.matchingComponentName treats a null
cname as "match every
- // component" - if this guard were skipped, the entire scoping
mechanism runScopedTestSuite
- // exists for would silently degrade to fully unscoped behavior
instead of erroring out.
- if (UtilValidate.isEmpty(fixedComponentName)) {
- return ServiceUtil.returnError("runScopedTestSuite requires a
fixed componentName");
- }
- Map<String, Object> scopedContext = new HashMap<>(context);
- scopedContext.put("componentName", fixedComponentName);
- return runTestSuite(dctx, scopedContext);
- }
-
/**
* Runs every ModelTestSuite the wrapper resolved (normally exactly one -
see
* JunitSuiteWrapper's suite-name filtering), reporting through a per-run
SuiteXmlReportWriter
@@ -488,39 +488,6 @@ public final class TestRunServices {
return result;
}
- /**
- * Runs {@link #getTestRunStatus} and, if the run exists, checks its
recorded componentName
- * against {@code expectedComponentName} - on a mismatch, returns the
exact same "No such runId"
- * error a genuinely unknown runId would produce (never a distinguishable
"wrong component"
- * message), so polling can't be used to detect the mere existence of
another component's runs. A
- * permission-denied result from the underlying call passes through
unchanged - the permission
- * check still runs first, exactly as it does for the unscoped
getTestRunStatus.
- * @param dctx the dispatch context
- * @param context the caller's service context
- * @param expectedComponentName the only component this call is allowed to
report on
- * @return the getTestRunStatus result, or a masked "No such runId" error
on a component mismatch
- */
- public static Map<String, Object> getScopedTestRunStatus(DispatchContext
dctx, Map<String, ?> context,
- String expectedComponentName) {
- // Fail closed, not open: an empty/null expectedComponentName must
never reach the
- // equality check below - unlike runScopedTestSuite's
fixedComponentName (which fails open
- // by matching every component), a null here would instead throw a raw
NullPointerException
- // out of expectedComponentName.equals(...), a different and equally
unacceptable failure
- // mode. Guard against both up front so this helper always fails the
same clean way.
- if (UtilValidate.isEmpty(expectedComponentName)) {
- return ServiceUtil.returnError("getScopedTestRunStatus requires an
expectedComponentName");
- }
- Map<String, Object> result = getTestRunStatus(dctx, context);
- if (ServiceUtil.isError(result)) {
- return result;
- }
- if (!expectedComponentName.equals(result.get("componentName"))) {
- String runId = (String) context.get("runId");
- return ServiceUtil.returnError("No such runId: " + runId);
- }
- return result;
- }
-
private static String readStringProperty(Delegator delegator, String
propertyName, String defaultValue) {
try {
String value = delegator == null
diff --git
a/framework/testtools/src/test/java/org/apache/ofbiz/testtools/TestRunServicesTest.java
b/framework/testtools/src/test/java/org/apache/ofbiz/testtools/TestRunServicesTest.java
index dc5ad8a20b..46eb5b05ba 100644
---
a/framework/testtools/src/test/java/org/apache/ofbiz/testtools/TestRunServicesTest.java
+++
b/framework/testtools/src/test/java/org/apache/ofbiz/testtools/TestRunServicesTest.java
@@ -85,7 +85,7 @@ class TestRunServicesTest {
Map.of("suiteName", "example-tests", "userLogin",
userLogin));
assertThat(result.get("responseMessage"), is("error"));
- assertThat(result.get("errorMessage"), is("The test execution API
is disabled in this environment (test.api.enabled=false)"));
+ assertThat(result.get("errorMessage"), is("The test execution API
is disabled in this environment."));
assertThat(result.get("runId"), nullValue());
}
}
@@ -136,180 +136,6 @@ class TestRunServicesTest {
assertThat(result.get("componentName"), is("example"));
}
- @Test
- void getScopedTestRunStatusReturnsRealDataWhenComponentMatches() {
- DispatchContext dctx = mock(DispatchContext.class);
- Security security = mock(Security.class);
- GenericValue userLogin = mock(GenericValue.class);
- when(dctx.getSecurity()).thenReturn(security);
- when(userLogin.getString("userLoginId")).thenReturn("admin");
- when(security.hasPermission("TESTEXEC_ADMIN",
userLogin)).thenReturn(true);
- TestRunServices.TRACKER.register("scoped-run-match", "example-tests",
"example", "admin", Map.of());
-
- Map<String, Object> result =
TestRunServices.getScopedTestRunStatus(dctx,
- Map.of("runId", "scoped-run-match", "userLogin", userLogin),
"example");
-
- assertThat(result.get("responseMessage"), is("success"));
- assertThat(result.get("componentName"), is("example"));
- }
-
- @Test
- void getScopedTestRunStatusMasksAMismatchedComponentAsUnknownRunId() {
- DispatchContext dctx = mock(DispatchContext.class);
- Security security = mock(Security.class);
- GenericValue userLogin = mock(GenericValue.class);
- when(dctx.getSecurity()).thenReturn(security);
- when(userLogin.getString("userLoginId")).thenReturn("admin");
- when(security.hasPermission("TESTEXEC_ADMIN",
userLogin)).thenReturn(true);
- TestRunServices.TRACKER.register("scoped-run-mismatch",
"content-tests", "content", "admin", Map.of());
-
- Map<String, Object> result =
TestRunServices.getScopedTestRunStatus(dctx,
- Map.of("runId", "scoped-run-mismatch", "userLogin",
userLogin), "example");
-
- assertThat(result.get("responseMessage"), is("error"));
- assertThat(result.get("errorMessage"), is("No such runId:
scoped-run-mismatch"));
- }
-
- @Test
- void getScopedTestRunStatusPassesThroughPermissionDenialUnchanged() {
- DispatchContext dctx = mock(DispatchContext.class);
- Security security = mock(Security.class);
- GenericValue userLogin = mock(GenericValue.class);
- when(dctx.getSecurity()).thenReturn(security);
- when(userLogin.getString("userLoginId")).thenReturn("nobody");
- when(security.hasPermission("TESTEXEC_ADMIN",
userLogin)).thenReturn(false);
-
- Map<String, Object> result =
TestRunServices.getScopedTestRunStatus(dctx,
- Map.of("runId", "any-run", "userLogin", userLogin), "example");
-
- assertThat(result.get("responseMessage"), is("error"));
- assertThat(result.get("errorMessage"), is("You do not have permission
to view test run status (TESTEXEC_ADMIN)"));
- }
-
- @Test
- void runScopedTestSuitePassesThroughPermissionDenialUnchanged() {
- // Cannot unit-test the componentName-forcing behavior itself in
isolation - like
- // runTestSuite's own suite-resolution path, that needs a real
bootstrapped ComponentConfig
- // (see this file's existing tests' pattern, and TestRunServices' own
"Design note on
- // testability"). This test only confirms the delegation is wired
correctly: a permission
- // denial passes straight through, and passing a deliberately
mismatched componentName
- // ("content") in the caller's context doesn't cause a crash before
the permission check
- // - proving nothing about whether the override happens, only that the
wrapper doesn't
- // reject/mangle the call. The override itself is verified by
manual/live validation (Task 5).
- DispatchContext dctx = mock(DispatchContext.class);
- Security security = mock(Security.class);
- Delegator delegator = mock(Delegator.class);
- GenericValue userLogin = mock(GenericValue.class);
- when(dctx.getSecurity()).thenReturn(security);
- when(dctx.getDelegator()).thenReturn(delegator);
- when(userLogin.getString("userLoginId")).thenReturn("nobody");
- when(security.hasPermission("TESTEXEC_ADMIN",
userLogin)).thenReturn(false);
-
- Map<String, Object> result = TestRunServices.runScopedTestSuite(dctx,
- Map.of("suiteName", "example-tests", "componentName",
"content", "userLogin", userLogin), "example");
-
- assertThat(result.get("responseMessage"), is("error"));
- assertThat(result.get("runId"), nullValue());
- }
-
- @Test
- void runScopedTestSuiteReturnsErrorForANullFixedComponentName() {
- // Must fail closed, not open: ComponentConfig.matchingComponentName
treats a null cname as
- // "match every component", so skipping this guard would silently turn
a null
- // fixedComponentName into fully unscoped behavior instead of an
error. Uses a permissive
- // security mock so the guard - not the permission check - is what's
actually exercised.
- DispatchContext dctx = mock(DispatchContext.class);
- Security security = mock(Security.class);
- Delegator delegator = mock(Delegator.class);
- GenericValue userLogin = mock(GenericValue.class);
- when(dctx.getSecurity()).thenReturn(security);
- when(dctx.getDelegator()).thenReturn(delegator);
- when(userLogin.getString("userLoginId")).thenReturn("admin");
- when(security.hasPermission("TESTEXEC_ADMIN",
userLogin)).thenReturn(true);
-
- Map<String, Object> result = TestRunServices.runScopedTestSuite(dctx,
- Map.of("suiteName", "example-tests", "userLogin", userLogin),
null);
-
- assertThat(result.get("responseMessage"), is("error"));
- assertThat(result.get("errorMessage"), is("runScopedTestSuite requires
a fixed componentName"));
- assertThat(result.get("runId"), nullValue());
- }
-
- @Test
- void runScopedTestSuiteReturnsErrorForAnEmptyFixedComponentName() {
- DispatchContext dctx = mock(DispatchContext.class);
- Security security = mock(Security.class);
- Delegator delegator = mock(Delegator.class);
- GenericValue userLogin = mock(GenericValue.class);
- when(dctx.getSecurity()).thenReturn(security);
- when(dctx.getDelegator()).thenReturn(delegator);
- when(userLogin.getString("userLoginId")).thenReturn("admin");
- when(security.hasPermission("TESTEXEC_ADMIN",
userLogin)).thenReturn(true);
-
- Map<String, Object> result = TestRunServices.runScopedTestSuite(dctx,
- Map.of("suiteName", "example-tests", "userLogin", userLogin),
"");
-
- assertThat(result.get("responseMessage"), is("error"));
- assertThat(result.get("errorMessage"), is("runScopedTestSuite requires
a fixed componentName"));
- assertThat(result.get("runId"), nullValue());
- }
-
- @Test
- void getScopedTestRunStatusReturnsErrorForANullExpectedComponentName() {
- // Without this guard, expectedComponentName.equals(...) would throw a
raw
- // NullPointerException instead of returning a clean error - a
different (and equally
- // unacceptable) failure mode than runScopedTestSuite's fail-open risk.
- DispatchContext dctx = mock(DispatchContext.class);
- Security security = mock(Security.class);
- GenericValue userLogin = mock(GenericValue.class);
- when(dctx.getSecurity()).thenReturn(security);
- when(userLogin.getString("userLoginId")).thenReturn("admin");
- when(security.hasPermission("TESTEXEC_ADMIN",
userLogin)).thenReturn(true);
-
- Map<String, Object> result =
TestRunServices.getScopedTestRunStatus(dctx,
- Map.of("runId", "any-run", "userLogin", userLogin), null);
-
- assertThat(result.get("responseMessage"), is("error"));
- assertThat(result.get("errorMessage"), is("getScopedTestRunStatus
requires an expectedComponentName"));
- }
-
- @Test
- void getScopedTestRunStatusReturnsErrorForAnEmptyExpectedComponentName() {
- DispatchContext dctx = mock(DispatchContext.class);
- Security security = mock(Security.class);
- GenericValue userLogin = mock(GenericValue.class);
- when(dctx.getSecurity()).thenReturn(security);
- when(userLogin.getString("userLoginId")).thenReturn("admin");
- when(security.hasPermission("TESTEXEC_ADMIN",
userLogin)).thenReturn(true);
-
- Map<String, Object> result =
TestRunServices.getScopedTestRunStatus(dctx,
- Map.of("runId", "any-run", "userLogin", userLogin), "");
-
- assertThat(result.get("responseMessage"), is("error"));
- assertThat(result.get("errorMessage"), is("getScopedTestRunStatus
requires an expectedComponentName"));
- }
-
- @Test
- void getScopedTestRunStatusMasksANullComponentNameRunAsUnknownRunId() {
- // Proves the fail-closed guarantee for a run registered with
componentName=null (e.g. a
- // hypothetical future unscoped internal registration path): the
scoped wrapper's equality
- // check must still deny it, returning the same masked "No such runId"
response a genuine
- // mismatch gets - never a crash, never real data.
- DispatchContext dctx = mock(DispatchContext.class);
- Security security = mock(Security.class);
- GenericValue userLogin = mock(GenericValue.class);
- when(dctx.getSecurity()).thenReturn(security);
- when(userLogin.getString("userLoginId")).thenReturn("admin");
- when(security.hasPermission("TESTEXEC_ADMIN",
userLogin)).thenReturn(true);
- TestRunServices.TRACKER.register("scoped-run-null-component",
"content-tests", null, "admin", Map.of());
-
- Map<String, Object> result =
TestRunServices.getScopedTestRunStatus(dctx,
- Map.of("runId", "scoped-run-null-component", "userLogin",
userLogin), "example");
-
- assertThat(result.get("responseMessage"), is("error"));
- assertThat(result.get("errorMessage"), is("No such runId:
scoped-run-null-component"));
- }
-
@Test
void runTestSuiteReturnsErrorWhenComponentApiDisabled() {
// Mocks EntityUtilProperties directly (rather than relying on the
classpath testtools.properties
@@ -339,15 +165,14 @@ class TestRunServicesTest {
assertThat(result.get("responseMessage"), is("error"));
assertThat(result.get("errorMessage"), is("The test execution API
is disabled for component 'example' "
- + "in this environment (test.api.enabled.example=false)"));
+ + "in this environment."));
assertThat(result.get("runId"), nullValue());
}
}
@Test
void runTestSuiteDoesNotRejectWhenComponentApiIsNotDisabled() {
- // Cannot verify a full successful run here - like
runScopedTestSuite's componentName-forcing
- // behavior (see
runScopedTestSuitePassesThroughPermissionDenialUnchanged above), resolving a
+ // Cannot verify a full successful run here - resolving a
// real suite needs a bootstrapped ComponentConfig this test module
doesn't have, so
// JunitSuiteWrapper's constructor still throws and this call still
ends in error - just not
// the new component-disabled error this test exists to rule out
(proving the gate was passed,
@@ -381,12 +206,19 @@ class TestRunServicesTest {
}
@Test
- void runTestSuiteSkipsComponentCheckWhenComponentNameIsBlank() {
+ void runTestSuiteRejectsBlankComponentName() {
// No componentName in context at all - mirrors
runTestSuiteReturnsErrorWhenApiDisabled's own
- // context map. The per-component override must never be consulted for
an unscoped call; this
- // proves EntityUtilProperties.getPropertyValue is never invoked with
a "test.api.enabled."-
- // prefixed property name (the global "test.api.enabled" key itself
does not match that prefix,
- // since it has no trailing dot).
+ // context map. Fail closed, not open: this is the only REST route to
runTestSuite (the generic
+ // framework/testtools/api/testruns.rest.xml endpoint), and REST
attribute binding merges
+ // body/path/query/header sources onto the same context map, so a
caller can still send an
+ // empty componentName (e.g. an empty query parameter) despite the
URL's path parameter
+ // normally supplying a real one. Without this guard, componentName
would reach
+ // ComponentConfig.matchingComponentName as null/blank - which matches
every component -
+ // silently turning the request into an unscoped sweep across every
component's tests. Also
+ // proves the per-component override is never even consulted for a
rejected call: verifies
+ // EntityUtilProperties.getPropertyValue is never invoked with a
"test.api.enabled."-prefixed
+ // property name (the global "test.api.enabled" key itself does not
match that prefix, since
+ // it has no trailing dot).
DispatchContext dctx = mock(DispatchContext.class);
Security security = mock(Security.class);
Delegator delegator = mock(Delegator.class);
@@ -401,10 +233,42 @@ class TestRunServicesTest {
entityUtilProperties.when(() ->
EntityUtilProperties.getPropertyValue("testtools", "test.api.enabled",
delegator))
.thenReturn("true");
- TestRunServices.runTestSuite(dctx, Map.of("suiteName",
"example-tests", "userLogin", userLogin));
+ Map<String, Object> result = TestRunServices.runTestSuite(dctx,
+ Map.of("suiteName", "example-tests", "userLogin",
userLogin));
+ assertThat(result.get("responseMessage"), is("error"));
+ assertThat(result.get("errorMessage"), is("runTestSuite requires a
componentName"));
+ assertThat(result.get("runId"), nullValue());
entityUtilProperties.verify(() ->
EntityUtilProperties.getPropertyValue(eq("testtools"),
startsWith("test.api.enabled."), eq(delegator)), never());
}
}
+
+ @Test
+ void runTestSuiteRejectsAnEmptyStringComponentName() {
+ // Distinct from the null/absent case above: an explicitly empty
string (e.g. what a REST
+ // caller sending "?componentName=" produces) must be rejected the
same way, not treated as
+ // "present" merely because the key exists in the context map.
+ DispatchContext dctx = mock(DispatchContext.class);
+ Security security = mock(Security.class);
+ Delegator delegator = mock(Delegator.class);
+ GenericValue userLogin = mock(GenericValue.class);
+ when(dctx.getSecurity()).thenReturn(security);
+ when(dctx.getDelegator()).thenReturn(delegator);
+ when(userLogin.getString("userLoginId")).thenReturn("admin");
+ when(security.hasPermission("TESTEXEC_ADMIN",
userLogin)).thenReturn(true);
+
+ try (MockedStatic<EntityUtilProperties> entityUtilProperties =
+ Mockito.mockStatic(EntityUtilProperties.class,
Mockito.CALLS_REAL_METHODS)) {
+ entityUtilProperties.when(() ->
EntityUtilProperties.getPropertyValue("testtools", "test.api.enabled",
delegator))
+ .thenReturn("true");
+
+ Map<String, Object> result = TestRunServices.runTestSuite(dctx,
+ Map.of("suiteName", "example-tests", "componentName", "",
"userLogin", userLogin));
+
+ assertThat(result.get("responseMessage"), is("error"));
+ assertThat(result.get("errorMessage"), is("runTestSuite requires a
componentName"));
+ assertThat(result.get("runId"), nullValue());
+ }
+ }
}
