Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix BWC test generation after mondernizing LineDocFile #13021

Merged
merged 5 commits into from
Jan 17, 2024

Conversation

s1monw
Copy link
Member

@s1monw s1monw commented Jan 17, 2024

The changes on #12929 broke the generation code for BWC indices since they are expecting certain fields created by LineDocFile. Yet, this change adds some sanity checks that run with unittest to ensure the bwc generatin is at least readable with the current version.

Relates to #12929

The changes on apache#12929 broke the generation code for BWC indices
since they are expecting vertain fields created by LineDocFile.
Yet, this change adds some sanity checks that run with unittest to ensure
the bwc generatin is at least readable with the current version.

Relates to apache#12929
@s1monw s1monw requested a review from jpountz January 17, 2024 12:46
Copy link
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

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

Thank you!

if (docIdDV == null) {
docIdDV = new NumericDocValuesField("docid_intDV", 0);
doc.add(docIdDV);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: this seems to assume that the Document instance that is returned is always the same, which feels a bit fragile. Maybe do a shallow copy of the document all the time? Or use TestUtil.cloneDocument()?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, so we are doing this in the other tests too bef0r my change. That's why i did it that way... I can clean this all up in a followup

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good!

@s1monw s1monw merged commit c661c88 into apache:main Jan 17, 2024
4 checks passed
s1monw added a commit that referenced this pull request Jan 17, 2024
The changes on #12929 broke the generation code for BWC indices
since they are expecting vertain fields created by LineDocFile.
Yet, this change adds some sanity checks that run with unittest to ensure
the BWC generation is at least readable with the current version.

Relates to #12929
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.

None yet

2 participants