You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "shubhamvishu (via GitHub)" <gi...@apache.org> on 2024/02/26 10:30:13 UTC
[PR] Fix TestLucene90FieldInfosFormat.testRandom [lucene]
shubhamvishu opened a new pull request, #13135:
URL: https://github.com/apache/lucene/pull/13135
### Description
Failing seed (`E9414A90E55BE2D`) generates empty string with `TestUtil#randomUnicodeString` whereas both field names must not be same as per [this](https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/index/FieldInfos.java#L399-L405). This fix adds an extra check that prevents this condition.
Test : passes test with failing seed
Closes #13129
<!--
If this is your first contribution to Lucene, please make sure you have reviewed the contribution guide.
https://github.com/apache/lucene/blob/main/CONTRIBUTING.md
-->
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix TestLucene90FieldInfosFormat.testRandom [lucene]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13135:
URL: https://github.com/apache/lucene/pull/13135#issuecomment-2030836068
This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution!
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix TestLucene90FieldInfosFormat.testRandom [lucene]
Posted by "stefanvodita (via GitHub)" <gi...@apache.org>.
stefanvodita merged PR #13135:
URL: https://github.com/apache/lucene/pull/13135
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix TestLucene90FieldInfosFormat.testRandom [lucene]
Posted by "shubhamvishu (via GitHub)" <gi...@apache.org>.
shubhamvishu commented on code in PR #13135:
URL: https://github.com/apache/lucene/pull/13135#discussion_r1528555456
##########
lucene/test-framework/src/java/org/apache/lucene/tests/index/BaseFieldInfoFormatTestCase.java:
##########
@@ -278,46 +278,50 @@ public void testRandom() throws Exception {
String parentField = random().nextBoolean() ? TestUtil.randomUnicodeString(random()) : null;
- var builder = INDEX_PACKAGE_ACCESS.newFieldInfosBuilder(softDeletesField, parentField);
-
- for (String field : fieldNames) {
- IndexableFieldType fieldType = randomFieldType(random(), field);
- boolean storeTermVectors = false;
- boolean storePayloads = false;
- boolean omitNorms = false;
- if (fieldType.indexOptions() != IndexOptions.NONE) {
- storeTermVectors = fieldType.storeTermVectors();
- omitNorms = fieldType.omitNorms();
- if (fieldType.indexOptions().compareTo(IndexOptions.DOCS_AND_FREQS_AND_POSITIONS) >= 0) {
- storePayloads = random().nextBoolean();
+ if (softDeletesField == null
Review Comment:
Your understanding is right, we are only fixing the edge case when both are empty and obviously not null.
> Would it be cleaner to add a loop generating a parentFieldName that is unique (or null)? Or maybe, if parentFIeld = softDeletesField, set it to null instead?
That seems fair to me, it'll be cleaner if we set one to null
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix TestLucene90FieldInfosFormat.testRandom [lucene]
Posted by "msokolov (via GitHub)" <gi...@apache.org>.
msokolov commented on code in PR #13135:
URL: https://github.com/apache/lucene/pull/13135#discussion_r1528450684
##########
lucene/test-framework/src/java/org/apache/lucene/tests/index/BaseFieldInfoFormatTestCase.java:
##########
@@ -278,46 +278,50 @@ public void testRandom() throws Exception {
String parentField = random().nextBoolean() ? TestUtil.randomUnicodeString(random()) : null;
- var builder = INDEX_PACKAGE_ACCESS.newFieldInfosBuilder(softDeletesField, parentField);
-
- for (String field : fieldNames) {
- IndexableFieldType fieldType = randomFieldType(random(), field);
- boolean storeTermVectors = false;
- boolean storePayloads = false;
- boolean omitNorms = false;
- if (fieldType.indexOptions() != IndexOptions.NONE) {
- storeTermVectors = fieldType.storeTermVectors();
- omitNorms = fieldType.omitNorms();
- if (fieldType.indexOptions().compareTo(IndexOptions.DOCS_AND_FREQS_AND_POSITIONS) >= 0) {
- storePayloads = random().nextBoolean();
+ if (softDeletesField == null
Review Comment:
this seems like somewhat complex logic here to handle a problem we created ( parentField name the same as softDeletesField name - empty?) Would it be cleaner to add a loop generating a parentFieldName that is unique (or null)? Or maybe, if parentFIeld = softDeletesField, set it to null instead?
Or - am I missing something here - did you fix some other potential edge cases too?
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Fix TestLucene90FieldInfosFormat.testRandom [lucene]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13135:
URL: https://github.com/apache/lucene/pull/13135#issuecomment-1989681283
This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution!
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org