Skip to content

Remove per-component test-run REST wrappers, superseded by the generic testtools endpoint - #374

Merged
ashishvijaywargiya merged 11 commits into
apache:trunkfrom
ashishvijaywargiya:rest-api-test-improvement
Aug 22, 2026
Merged

Remove per-component test-run REST wrappers, superseded by the generic testtools endpoint#374
ashishvijaywargiya merged 11 commits into
apache:trunkfrom
ashishvijaywargiya:rest-api-test-improvement

Conversation

@ashishvijaywargiya

@ashishvijaywargiya ashishvijaywargiya commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Title: Remove per-component test-run REST wrappers, superseded by the generic testtools endpoint

Deletes the per-component test-run REST wrapper pattern from example, ecommerce, assetmaint,
lucene, and scrum:

  • the groovy wrapper class in each component (e.g. ExampleTestRunServices.groovy)
  • the two duplicated definitions each wrapper needed, in each component's
    servicedef/services.xml
  • each component's own test-run *.rest.xml (for example, this is the TestRunResource block
    removed from its shared rest.xml, since that file also carries example's own business-service
    REST resource; for the other four it is the entire file, since they had nothing else in it)

ecommerce's servicedef/services.xml and its ofbiz-component.xml service-resource entry are
removed entirely - that file existed solely to declare the two now-deleted services.

Why

Superseded by a new, generic, framework-owned REST endpoint added in the companion
ofbiz-framework pull request, which takes componentName as a URL path parameter and needs no
per-component files at all. All five components' tests remain reachable, now via

POST /rest/testtools/testruns/{componentName}
GET /rest/testtools/testruns/{runId}

instead of each component's own separately-branded URL.

No changes to any component's actual test suites, Jupiter test classes, or testdef files - this
is REST/service wiring only.

Testing

  • gradlew classes testClasses passes after each component's change.
  • Full sweep against a running server: all five old branded URLs (example-rest, ecommerce-rest,
    assetmaint-rest, lucene-rest, scrum-rest) confirmed gone (404); all five components' real test
    suites confirmed passing end-to-end through the new generic endpoint.
  • 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 before, the companion ofbiz-framework pull request (apache/ofbiz-framework#1699). This PR
removes the last callers of TestRunServices.runScopedTestSuite/getScopedTestRunStatus, which the
framework PR deletes. Merging the framework PR first while these wrapper scripts are still
present would leave them calling methods that no longer exist.

Adds runEcommerceTestSuite/getEcommerceTestRunStatus services, wrapping
TestRunServices.runScopedTestSuite/getScopedTestRunStatus and fixed to the
ecommerce component only, following the same pattern already used by the
example component. Exposes them via a new ecommerce-tests.rest.xml REST
resource (POST/GET /rest/ecommerce-rest/testruns).

Also parameterizes OrderNotificationTests.groovy's constants (orderId,
sendTo, note, comments) through testParams with the existing literal
values as defaults, so a caller can override them per test run without
changing any test source.
Adds runAssetmaintTestSuite/getAssetmaintTestRunStatus services, wrapping
TestRunServices.runScopedTestSuite/getScopedTestRunStatus and fixed to the
assetmaint component only, following the same pattern already used by
example and ecommerce. Exposes them via a new assetmaint-tests.rest.xml
REST resource (POST/GET /rest/assetmaint-rest/testruns).

Also parameterizes FixedAssetMaintTests.groovy's constants (fixedAssetId,
statusId, productMaintSeqId/productMaintTypeId, intervalMeterTypeId, the
three creation dates, the update-phase completion date, and the final
completed status) through testParams with the existing literal values as
defaults, so a caller can override them per test run without changing any
test source.
Adds runLuceneTestSuite/getLuceneTestRunStatus services, wrapping
TestRunServices.runScopedTestSuite/getScopedTestRunStatus and fixed to the
lucene component only, following the same pattern already used by example,
ecommerce, and assetmaint. Exposes them via a new lucene-tests.rest.xml
REST resource (POST/GET /rest/lucene-rest/testruns).

Also parameterizes LuceneTests.groovy's constants (contentId, queryLine)
through testParams with the existing literal values as defaults, so a
caller can override them per test run without changing any test source.
Adds runScrumTestSuite/getScrumTestRunStatus services, wrapping
TestRunServices.runScopedTestSuite/getScopedTestRunStatus and fixed to the
scrum component only, following the same pattern already used by example,
ecommerce, assetmaint, and lucene. Exposes them via a new
scrum-tests.rest.xml REST resource (POST/GET /rest/scrum-rest/testruns).

scrum's testdef uses <test-group> instead of <test-case> for its 10
Jupiter test classes; verified this resolves identically for suiteName/
testCaseName/testMethodName purposes since each group wraps exactly one
jupiter-test-suite child.

testParams parameterization of the 10 individual test classes is tracked
separately.
Parameterizes every constant across DailyMeetingMinuteTests, MyWorkTests,
ProductBacklogTests, ProductTests, ScrumProjectTests, ScrumTests,
SprintBacklogTests, SprintTests, TaskTests, and TimesheetTests through
testParams, with each method's existing literal values as defaults - same
mechanical pattern already applied to ecommerce, assetmaint, and lucene.
Rest of each method's logic (service calls, assertions, control flow) is
unchanged; assertions that previously compared against a hardcoded literal
now compare against the same now-parameterized local variable so overriding
a value doesn't desync the assertion from the input.

Full suite (41/41) verified passing with default testParams via the
runScrumTestSuite REST endpoint added in the prior commit; a testParams
override was also verified to correctly propagate through and change
service-call behavior.
…arCloud

FixedAssetMaintTests, MyWorkTests, ProductBacklogTests, and ScrumProjectTests each had
multiple @test methods with near-identical bodies, differing only in one or two literal
values (a mode flag, a status flag, or a set of fixture IDs). Extracted a private helper
per class that takes the differing value(s) as parameters and does the shared
testParams-resolution/service-call/assertion work; each @test method is now a one-line
call into its class's helper.

No behavior change: every test method remains independently defined and selectable
(unaffected by this for REST testMethodName targeting), testParams override behavior is
identical (each helper still resolves testParams.<key> ?: <that test's own literal
default>), and default values are unchanged. Verified via the generic test-run endpoint:
assetmainttests 3/3 and scrumtests 41/41 both still pass with the same counts as before.
@sonarqubecloud

Copy link
Copy Markdown

@ashishvijaywargiya
ashishvijaywargiya merged commit 4c9f732 into apache:trunk Aug 22, 2026
2 checks passed
ashishvijaywargiya added a commit to apache/ofbiz-framework that referenced this pull request Aug 22, 2026
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.
@ashishvijaywargiya
ashishvijaywargiya deleted the rest-api-test-improvement branch August 22, 2026 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant