[Tests] Fix RequestTests NPE #28391
[Tests] Fix RequestTests NPE #28391
Conversation
cbuescher
added
>test
|
I think this cannot happen on 6.x and master because we already use SearchRequest(indices) there. However I was wondering if we can get rid of the empty ctor or set the source to something other than null in that case. Maybe something for a follow up PR. |
javanna
left a comment
javanna
left a comment
There was a problem hiding this comment.
LGTM . Given that the SearchRequest empty constructor is public, maybe we should have a small test that verifies that things don't break when the source is null. I am not sure that is tested once this change is made. (that should also be applied to master and 6.x).
|
Will need to fix CI failure first... |
Previuosly SearchRequest#source would rarely be null because the empty constructor was used. We shouldn't use that constructor in the test. Closes elastic#28388
cbuescher
force-pushed
the
fix-requestTests-NPE
branch
from
ba345de to
325b187
Compare
|
CI still fails because our PR testing infra currently builds pull request against 5.6 with the wrong JDK (9 instead of 8). However, the changed test passes locally and also in CI, so I'm going to merge this. |
|
Still failing on 6.1 (https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+6.1+multijob-unix-compatibility/os=debian/266/console) so I'm going to port it there and also check the other 6.x branches again if this can happen there. |
Previuosly SearchRequest#source would rarely be null because the empty constructor was used. We shouldn't use that constructor in the test. Closes #28388
Previuosly SearchRequest#source would rarely be null because the empty constructor was used. We shouldn't use that constructor in the test. Closes #28388
3 participants
Previuosly SearchRequest#source would rarely be null because the empty
constructor was used. We shouldn't use that constructor in the test.
Closes #28388