adamsaghy commented on PR #6271: URL: https://github.com/apache/fineract/pull/6271#issuecomment-5314185767
> Thanks for picking this up, @adamsaghy — front-loading the job-heavy classes is a sensible lever, and it's nice that `scripts/split-tests.sh` already understands class-level `@Order` so the annotated classes get spread round-robin across the 15 shards. > > I've left some questions inline, mostly trying to understand the intent rather than pushing for changes. A few that don't attach to a particular line: > > 1. **Measured effect** — could [FINERACT-2684](https://issues.apache.org/jira/browse/FINERACT-2684) record the before/after numbers? Which shard was the bottleneck, and how much wall-clock does this save? The green matrix here doesn't really demonstrate it, since each shard runs serially either way. > 2. **Scaling** — ordering only takes effect within a shard's JVM. At 30 ordered classes over 15 shards it happens to land at exactly 2 per shard; what happens at 31, or if `total_shards` changes? Would it be worth having `split-tests.sh` print the ordered-class distribution so drift becomes visible in the logs? > 3. **Parsing robustness** — the awk in `split-tests.sh` matches the literal `@Order(<digits>)` form. If someone writes `@Order(value = 1)`, JUnit still honours it but the splitter silently treats the class as unordered. Worth a guard, or is the shorthand form enough of a convention? > 4. **Scope** — would splitting this into three PRs be practical (the `@Order` annotations / the `SavingsTestLifecycleExtension` rollout / the `enable-auto-generated-external-id` fix in `LoanManualInterestRefundResponseStructureTest`)? The third is a behavioural change that's easy to miss inside a test-infra change. > > Nothing blocking from me — just comments. Thanks again! 1.Execution times of the chunks are always logged 2. Over scaling does not bring any value + github and Apache infra has their own limitations. 3. let me review whether we can enhance further the logic 4. I would not split them. These issues were bring into light during the reordering. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
