janhoy opened a new pull request, #102:
URL: https://github.com/apache/solr-orbit/pull/102
### Description
`--test-iterations=1` ran no benchmark, printed `✅ SUCCESS`, and exited 0.
`dispatch_sub_command` computes `iterations = int(args.test_iterations)` and
branches on `iterations > 1`, but the single-run branch tested
`args.test_iterations == 1` — the raw argparse value. The argument has
`default=1` and no `type=`, so it is the integer `1` when defaulted and the
string `"1"` when passed on the command line. `"1" == 1` is false, so an
explicit `--test-iterations=1` fell through to the `else` and printed "Please
enter a valid number of test iterations" while still exiting 0.
Two lines:
- compare the parsed `iterations`, so the value's origin stops mattering;
- give the argument `type=positive_number` (the helper already in that
parser, used by `--runtime-jdk`), so `args.test_iterations` is always an `int`
and an invalid value becomes an argparse error with a usage message rather than
an `int()` traceback further in.
### Issues Resolved
Closes #101
### Testing
- [ ] New functionality includes testing
Before, on `main`:
```
$ solr-orbit run --pipeline=benchmark-only --target-hosts=127.0.0.1:8983 \
--workload=nyc_taxis --test-iterations=1 --test-mode
[INFO] Please enter a valid number of test iterations
[INFO] ✅ SUCCESS (took 0 seconds)
```
After, the same command runs the benchmark and reports a test-run id.
Invalid values are now rejected up front:
```
$ solr-orbit run --test-iterations=0 ...
argument --test-iterations: must be positive but was 0
$ solr-orbit run --test-iterations=abc ...
argument --test-iterations: invalid positive_number value: 'abc'
```
`make lint` and `make test` pass (1132 passed, 5 skipped).
No unit test added: the bug is in `dispatch_sub_command`, which is a long
`if/elif` chain over `args` that runs the real pipeline, so covering it needs
either a refactor to make the dispatch testable or an integration test. The
`it/` suite would be the right home, but it is currently failing for an
unrelated reason that I will raise separately. Happy to add a test here if you
would like one in a particular form.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
