Skip to content

Conversation

@Farhantigadi
Copy link
Contributor

The split-tests.sh script currently only checks if arguments are provided but doesn't validate their values. This can lead to confusing behavior when users pass invalid inputs like negative numbers, zero, or non-numeric values.

This change adds basic input validation to make the script more robust and user-friendly. Now when someone runs the script with invalid arguments (like ./split-tests.sh 0 5 or ./split-tests.sh 3 abc), they'll get a clear error message explaining what went wrong instead of the script failing mysteriously later.

The validation ensures:

Both arguments are positive integers (no zero, negative, or text values)

The shard index doesn't exceed the total number of shards

Users get helpful error messages when something is wrong

This is a small improvement that makes the script safer and easier to use, especially for new contributors who might not be familiar with the expected input format.

@IOhacker
Copy link
Contributor

IOhacker commented Jan 7, 2026

Please add this Jira Ticket on the commit and the PR "FINERACT-2434"
Please help us to open Jira Tickets for any PR https://issues.apache.org/jira/browse/FINERACT-2434

Copy link
Contributor

@IOhacker IOhacker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kindly see my comments

@Farhantigadi Farhantigadi force-pushed the feature/split-tests-args-validation branch from d1cfa86 to 99a0f3d Compare January 7, 2026 17:21
@Farhantigadi Farhantigadi changed the title Add input validation to split-tests.sh script FINERACT-2434: Add input validation to split-tests.sh script Jan 7, 2026
@Farhantigadi Farhantigadi requested a review from IOhacker January 7, 2026 17:31
@fintecheando
Copy link

LGTM

Copy link
Contributor

@Aman-Mittal Aman-Mittal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@IOhacker IOhacker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@IOhacker IOhacker merged commit 4c1016d into apache:develop Jan 8, 2026
34 checks passed
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.

4 participants