-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Super randomized tests for fetch fields API #70278
Changes from 21 commits
233be6b
7bf4d60
26033ca
24a9062
3802c55
6a3696d
e03b5a5
3e5fc11
a7f0b29
a3d9402
821f3a6
9ab9151
b37d90c
80d3e20
75f4f0c
d7bfa96
43f9df6
e9bbb22
c712c6f
08224f6
9ebe3b8
ff51867
7ad821d
463171e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ | |
import java.util.Arrays; | ||
import java.util.Collection; | ||
import java.util.List; | ||
import java.util.function.Supplier; | ||
|
||
import static org.hamcrest.Matchers.instanceOf; | ||
|
||
|
@@ -136,4 +137,10 @@ public void testRejectMultiValuedFields() throws MapperParsingException, IOExcep | |
assertEquals("[rank_feature] fields do not support indexing multiple values for the same field [foo.field] in the same document", | ||
e.getCause().getMessage()); | ||
} | ||
|
||
@Override | ||
protected Supplier<? extends Object> randomFetchTestValueVendor(MappedFieldType ft) { | ||
assumeFalse("Test implemented in a follow up", true); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did this because I wanted to be able to open the PR and for everything to compile. I do expect to handle this in a follow up. |
||
return null; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,8 +18,10 @@ | |
import org.elasticsearch.common.xcontent.XContentBuilder; | ||
import org.elasticsearch.common.xcontent.XContentFactory; | ||
import org.elasticsearch.index.mapper.ParseContext.Document; | ||
import org.elasticsearch.test.ESTestCase; | ||
|
||
import java.io.IOException; | ||
import java.util.function.Supplier; | ||
|
||
public class BooleanFieldMapperTests extends MapperTestCase { | ||
|
||
|
@@ -162,4 +164,20 @@ public void testDocValues() throws Exception { | |
assertEquals(DocValuesType.NONE, fields[0].fieldType().docValuesType()); | ||
assertEquals(DocValuesType.SORTED_NUMERIC, fields[1].fieldType().docValuesType()); | ||
} | ||
|
||
@Override | ||
protected Supplier<? extends Object> randomFetchTestValueVendor(MappedFieldType ft) { | ||
switch (between(0, 3)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will definitely be nicer when it doesn't return a |
||
case 0: | ||
return ESTestCase::randomBoolean; | ||
case 1: | ||
return () -> randomBoolean() ? "true" : "false"; | ||
case 2: | ||
return () -> randomBoolean() ? "true" : ""; | ||
case 3: | ||
return () -> randomBoolean() ? "true" : null; | ||
default: | ||
throw new IllegalStateException(); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I chose a
Supplier
here because we have a test that tests many values at once and I wanted to be able to build many values that have a similar "theme". Check out the tests for dates for an example of where this is useful.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you explain this more? I looked at the date tests but couldn't tell where having a 'supplier' was critical.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rereading this I don't think it really is critical. It was when was sorting the values and deduping them, but it isn't really critical any more. It is a bit more realistic to generate similarly themed values though. But we don't really need it any more. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to making this just an
Object