[sort-coverage] Remove (again) 6 occurrences of `if step(a::Range{T} == 0`?
Ismael VC
Continuing my quest to enhance test coverage, I found 6 occurrences of this test: if step(a::Range{T} == 0 in test/sort.jl, that are never exercised in the tests:
There are no tests for this, but also:
julia> 1:0:5 # v0.4.0-dev+5149
ERROR: ArgumentError: step cannot be zero
in steprange_last at ./range.jl:29
in colon at ./range.jl:96
I also found two related commits:
Boundschecks for searchsorted & remove step(r::Range)==0 tests, which removed the tests.
Revert “Boundschecks for searchsorted & remove step(r::Range)==0 tests”, which re-added the tests.
It seems to me we don’t really know why they are there, since ranges can’t have a step of 0:
At the time this was committed, I think that ranges could actually
have a zero step – there was a reason for that check to be there.
This may no longer be the case, but since it’s cheap to check, we may
as well handle it correctly and decouple the correctness of the
sorting functions from a detail of the range implementation.
Even though function correctness is decoupled from the range implementation, IMHO the tests in the functions are not correct, it doesn’t mater that this test is cheap, it will never run as is, I’ve just checked against v0.2.1 and we couldn’t make ranges with step 0 even back then, is this a planned addition?, or can I issue a PR to remove this tests?
julia> step(1:0:5) # v0.2.1
ERROR: step cannot be zero in colon syntax
in colon at range.jl:35
This is all that it’s stopping this file from having 100% coverage.
Ismael VC
I’ve already opened PR #11799 in case, this is fine.