-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Add elasticsearch repository test #679
Add elasticsearch repository test #679
Conversation
@sothawo what do you think? |
I will have a look, but cannot do this before the next weekend |
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 left a couple of comments
result.forEach(it -> verify(it, expectedWord, expectedDate)); | ||
} | ||
|
||
private void verify(Conference it, String expectedWord, LocalDate expectedDate) { |
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'd inline this method
@@ -51,16 +55,16 @@ public void insertDataSample() { | |||
// Save data sample | |||
|
|||
var documents = Arrays.asList( | |||
Conference.builder().date("2014-11-06").name("Spring eXchange 2014 - London") | |||
Conference.builder().date(LocalDate.parse("2014-11-06", FORMAT)).name("Spring eXchange 2014 - London") |
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.
instead of parsing, wouldn't it be better to use
LocalDate.of(2014,11,6)
protected static final DateTimeFormatter FORMAT = DateTimeFormatter.ISO_LOCAL_DATE; | ||
|
||
private static final ElasticsearchContainer CONTAINER = new ElasticsearchContainer( | ||
DockerImageName.parse("docker.elastic.co/elasticsearch/elasticsearch:8.7.0")) // |
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.
The current released version of Spring Data Elasticsearch is built against Elasticsearch 8.13.4 so. would at least use that.
@Test | ||
void textSearch() { | ||
|
||
var expectedDate = LocalDate.parse("2014-10-29", FORMAT); |
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.
as above
LocalDate.of(2014, 9, 29)
protected static final DateTimeFormatter FORMAT = DateTimeFormatter.ISO_LOCAL_DATE; | ||
|
||
private static final ElasticsearchContainer CONTAINER = new ElasticsearchContainer( | ||
DockerImageName.parse("docker.elastic.co/elasticsearch/elasticsearch:8.7.0")) // |
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.
see imperative code
Closing as this pull request wasn't updated in time. If you would like us to look at this issue, please provide additional information and we will re-open the issue. |
LocalDate
rather thanString
type to match the annotation@Field(type = Date)
. The formerString
type is a misuse of date type and will cause the following warn: