Add Reverse ordering support for Task API#816
Add Reverse ordering support for Task API#816meili-bors[bot] merged 10 commits intomeilisearch:mainfrom
Conversation
|
Hi @ma1581, thank you for your PR. 🙌 I merged the changes from the Thanks! |
|
Hi @Strift . thanks for notifying this. Glad to be part of this |
brunoocasali
left a comment
brunoocasali
left a comment
There was a problem hiding this comment.
Thanks a lot for your PR!
Next time I would ask you to make one change per PR, it makes it easier to review :)
| @BeforeEach | ||
| void beforeEach() { | ||
| TimeZone.setDefault(TimeZone.getTimeZone("UTC")); | ||
| } | ||
|
|
There was a problem hiding this comment.
Why did you need to set this? And if "yes" why not to set this for the whole test suite?
There was a problem hiding this comment.
this is not necessary entirely, as it works find even without it. but when running the test cases locally test cases related to dates, they are bound to fail.
this will allow devs to be sure that no False negative happens when testing locally.
if my above reasoning is fine, will apply to this to entire test suite in a new commit
There was a problem hiding this comment.
Yes please add in a new PR :)
| Task[] defaultTaskList = | ||
| client.getTasks(new TasksQuery().setAfterEnqueuedAt(currentTime)).getResults(); | ||
| Task[] reversedTaskList = | ||
| client.getTasks(new TasksQuery().setAfterEnqueuedAt(currentTime).setReverse(true)) | ||
| .getResults(); |
There was a problem hiding this comment.
Can you map only the task ids here, and then you assert them later, so the test will be less complex to read.
| Arrays.stream(defaultTaskList).map(Task::getUid).collect(Collectors.toList()); | ||
| List<Integer> reversedTaskOrder = | ||
| Arrays.stream(reversedTaskList).map(Task::getUid).collect(Collectors.toList()); | ||
| assertFalse(originalTaskOrder.isEmpty()); |
There was a problem hiding this comment.
You can also remove one of these assertions.
There was a problem hiding this comment.
removed assertFalse(reversedTaskOrder.isEmpty()
|
Hi @brunoocasali . |
|
bors merge |
|
Build succeeded: |
4 participants
Pull Request
Related issue
Fixes #798
What does this PR do?
PR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!