Skip to content

[test] index test updates OP#318 #6033

Merged
line-o merged 6 commits intoeXist-db:developfrom
duncdrum:test-lucene
Feb 11, 2026
Merged

[test] index test updates OP#318 #6033
line-o merged 6 commits intoeXist-db:developfrom
duncdrum:test-lucene

Conversation

@duncdrum
Copy link
Contributor

@duncdrum duncdrum commented Feb 4, 2026

while going though old issue and checking their test coverage I found the mix of existing xqsuite test in xml testSet format, and xquery format irritating. This now unifies on XQ format, for easier portability, and better test isolation. I could also unskip a few test which hadn't been running since the spice girls, but which are now passing. All test from xml have been refactored keeping their syntax and intention as intact as possible.

What actually go me going are the first 4 commits. These are pending tests for open issue, confirmed to still exist in our code base since long before the spice girls.

I expect further refactoring of the indexing tests and more additions, so it would be good to get this merged as the basis for that. Focusing on:

  • older test relying on assert true where more expressive assertions, makes it easier to see what went wrong when something does go wrong.
  • More open issues I still have to reproduce and see if we should add pending tests for them
  • Some cleanup and shuffling of redundant tests where xml and xq were testing the same thing.

see #2393

@duncdrum duncdrum added the Lucene issue is related to Lucene or its integration label Feb 4, 2026
@duncdrum duncdrum added this to the eXist-7.0.0 milestone Feb 5, 2026
@adamretter
Copy link
Contributor

adamretter commented Feb 5, 2026

In terms of debugging tests and fixing issues in the actual underlying Java code, this is a bad idea. XQSuite is horribly convoluted when trying to understand the real problems that are in the Java code. This will make the developers work much harder. You would have been better going in the opposite direction and converting the XQSuite tests into the older XML format, it's runner implement is much simpler and much easier to debug the Java code when it executes

unskip ~17 tests which are passing since they were added

decouple tests from classpath based sample jar

increase test isolation to avoid setUp tearDown from differnt specs interfering with each other
so they no longer depend on state from xml testset
@duncdrum duncdrum marked this pull request as ready for review February 5, 2026 18:31
@duncdrum duncdrum requested a review from a team as a code owner February 5, 2026 18:31
@duncdrum
Copy link
Contributor Author

duncdrum commented Feb 5, 2026

execution times for last test run on linux develop and here are 2:14m and 2:15m for the lucene tests. So with slightly more tests performance hasn't suffered.

Turns out some of the existing tests were relying on state from the xml specs or from previous tests, which mavend exaggerated on CI. Sharing state does not strike me as conducive to a good developer experience.

@adamretter the horrible convolutedness of a mix of xml and xq tests while working on the underlying java code is what motivated this PR. The work won't stop here.

@line-o
Copy link
Member

line-o commented Feb 5, 2026

I do appreciate the effort you poured into this @duncdrum !
Especially the tests that aim to give more meaningful assertion errors would certainly benefit from the ability to create custom assertions that we will get once my feature PR #4332 is merged.

@adamretter
Copy link
Contributor

adamretter the horrible convolutedness of a mix of xml and xq tests while working on the underlying java code is what motivated this PR. The work won't stop here.

But switching to XQSuite makes this much much worse. Are you doing to instead switch all the tests to the XML format instead then to improve this?

: @param $group group name: smp-indexed | smp-dropped | sip-indexed | sip-dropped
: @return xs:string "group: count" e.g. "smp-indexed: 5"
:)
declare function unic-smp-l:lucene-finds-count($group as xs:string) as xs:string {
Copy link
Member

Choose a reason for hiding this comment

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

This here could be rewritten to a custom assertion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes and it might in a future refactoring of the lucene tests, but this is the closest to the issue report. So I d rather leave this commit as is and focus on a larger refactoring of the entire Lucene suite, including this spec.

@duncdrum duncdrum changed the title [test] index test updates [test] index test updates OP#318 Feb 6, 2026
Copy link
Member

@dizzzz dizzzz left a comment

Choose a reason for hiding this comment

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

LGTM ; nice job

@line-o line-o self-requested a review February 10, 2026 11:02
Copy link
Member

@line-o line-o left a comment

Choose a reason for hiding this comment

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

(sorry that is not the case) ✂️

@duncdrum
Copy link
Contributor Author

@line-o so good to merge then?

@line-o line-o merged commit 13c5c90 into eXist-db:develop Feb 11, 2026
12 of 13 checks passed
@duncdrum duncdrum deleted the test-lucene branch February 11, 2026 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Lucene issue is related to Lucene or its integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants