[bugfix] tails of subsequences off by one #4850
Conversation
|
Wow, 17 additional XQTS test pass! |
|
Quite a reward for removing a few characters from such a large codebase. |
|
rebased onto current develop |
|
@line-o thx looks good to me |
| @@ -204,7 +204,7 @@ public Sequence tail() { | |||
| return Sequence.EMPTY_SEQUENCE; | |||
| } | |||
|
|
|||
| return new SubSequence(fromInclusive + 1, toExclusive -1, sequence); | |||
| return new SubSequence(fromInclusive + 1, toExclusive, sequence); | |||
| %test:args(1,5) | ||
| %test:assertEquals(2,3,4,5) | ||
| function ss:tail($start, $length) { | ||
| subsequence((1 to 5), $start, $length) => tail() |
There was a problem hiding this comment.
Can you remove the unnecessary Arrow Operator please? For the reasons:
- As previously discussed the ability to debug such constructs within XQuery from Java is not at all easy.
- It is not at all required, the test only needs the two functions:
tail(subsequence((1 to 5), $start, $length)). Ideally tests should only have the required components.
There was a problem hiding this comment.
Do you really think this warrants to reject this PR?
There was a problem hiding this comment.
The existing XQSuite test case above the newly added one has an arrow operator as well.
I also do used this new test case to debug and fix the issue. It is certainly possible.
There was a problem hiding this comment.
his warrants to reject this PR?
I am not rejecting the PR, I am asking for a very small change that has a positive impact. Surely the purpose of the peer-review process is exactly for this purpose.
The existing XQSuite test case above the newly added one has an arrow operator as well.
I don't see what bearing that has on your changes. If the existing code was perfect, then we would never need to make changes to eXist-db! When we are making changes, these days, we follow the peer-review process, which hopefully leads to agreed improvements to the code base.
|
For the sake of getting this PR, and it's backport #4851, merged I will change the test. |
|
Kudos, SonarCloud Quality Gate passed! |
Description:
Tails of subsequences are no longer off by one.
Reference:
fixes #4830
Type of tests:
XQSuite tests