You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "easyice (via GitHub)" <gi...@apache.org> on 2024/02/14 17:52:23 UTC
[PR] Fix NPE for TestRandomChains.testRandomChainsWithLargeStrings [lucene]
easyice opened a new pull request, #13104:
URL: https://github.com/apache/lucene/pull/13104
Related to LUCENE-10353
```
./gradlew test --tests TestRandomChains.testRandomChainsWithLargeStrings -Dtests.seed=1B915D6C476F9BFE -Dtests.nightly=true -Dtests.locale=mn -Dtests.timezone=Canada/Mountain -Dtests.asserts=true -Dtests.file.encoding=UTF-8
```
```
org.apache.lucene.analysis.tests.TestRandomChains > testRandomChainsWithLargeStrings FAILED
java.lang.NullPointerException: Cannot invoke "String.equals(Object)" because the return value of "org.apache.lucene.analysis.tokenattributes.TypeAttribute.type()" is null
at __randomizedtesting.SeedInfo.seed([1B915D6C476F9BFE:71CAE27D1E21BB0D]:0)
at org.apache.lucene.analysis.common@10.0.0-SNAPSHOT/org.apache.lucene.analysis.payloads.NumericPayloadTokenFilter.incrementToken(NumericPayloadTokenFilter.java:49)
at org.apache.lucene.analysis.common@10.0.0-SNAPSHOT/org.apache.lucene.analysis.miscellaneous.ConditionalTokenFilter.incrementToken(ConditionalTokenFilter.java:185)
at org.apache.lucene.test_framework@10.0.0-SNAPSHOT/org.apache.lucene.tests.analysis.ValidatingTokenFilter.incrementToken(ValidatingTokenFilter.java:81)
at org.apache.lucene.analysis.common@10.0.0-SNAPSHOT/org.apache.lucene.analysis.miscellaneous.ConditionalTokenFilter$OneTimeWrapper.incrementToken(ConditionalTokenFilter.java:65)
at org.apache.lucene.analysis.icu@10.0.0-SNAPSHOT/org.apache.lucene.analysis.icu.ICUNormalizer2Filter.incrementToken(ICUNormalizer2Filter.java:80)
at org.apache.lucene.analysis.common@10.0.0-SNAPSHOT/org.apache.lucene.analysis.miscellaneous.ConditionalTokenFilter.incrementToken(ConditionalTokenFilter.java:203)
at org.apache.lucene.test_framework@10.0.0-SNAPSHOT/org.apache.lucene.tests.analysis.ValidatingTokenFilter.incrementToken(ValidatingTokenFilter.java:81)
at org.apache.lucene.test_framework@10.0.0-SNAPSHOT/org.apache.lucene.tests.analysis.BaseTokenStreamTestCase.checkResetException(BaseTokenStreamTestCase.java:926)
at org.apache.lucene.test_framework@10.0.0-SNAPSHOT/org.apache.lucene.tests.analysis.BaseTokenStreamTestCase.checkRandomData(BaseTokenStreamTestCase.java:1068)
at org.apache.lucene.analysis.tests@10.0.0-SNAPSHOT/org.apache.lucene.analysis.tests.TestRandomChains.testRandomChainsWithLargeStrings(TestRandomChains.java:978)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
```
--
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 NPE for TestRandomChains.testRandomChainsWithLargeStrings [lucene]
Posted by "rmuir (via GitHub)" <gi...@apache.org>.
rmuir commented on PR #13104:
URL: https://github.com/apache/lucene/pull/13104#issuecomment-1944453765
good catch! should we move this check to `setTokenType()`? Since it is a `public` method, it seems like a better place to add the check?
--
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 NPE for TestRandomChains.testRandomChainsWithLargeStrings [lucene]
Posted by "easyice (via GitHub)" <gi...@apache.org>.
easyice commented on PR #13104:
URL: https://github.com/apache/lucene/pull/13104#issuecomment-1946004282
Thank you for the great suggestions!
@uschindler You are right, all the setters are used after the construction only, It makes sense if they are only set on construction. The only thing to consider is the impact of API changes to the users. for the time being I follow the @rmuir 's suggestion, move this check to `setTokenType`.
--
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 NPE for TestRandomChains.testRandomChainsWithLargeStrings [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #13104:
URL: https://github.com/apache/lucene/pull/13104#issuecomment-1944583551
I wonder why the token stream has a setter at all. The field should be final.
I thought we want to have those settings all final and only set on construction.
--
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 NPE for TestRandomChains.testRandomChainsWithLargeStrings [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #13104:
URL: https://github.com/apache/lucene/pull/13104#issuecomment-1946149224
Let's open another issue about this. I think we did the final only fields for query instances, but token streams are not behaving bad as they are not cached and have state.
But we should avoid that in future. There is no good reason to change config of token streams after construction.
--
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 NPE for TestRandomChains.testRandomChainsWithLargeStrings [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler merged PR #13104:
URL: https://github.com/apache/lucene/pull/13104
--
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