Add integration tests, Docker on CI, and implement waitForPendingUpdate #32
Conversation
eskombro
force-pushed
the
add_integration_tests
branch
from
2642014 to
412140e
Compare
eskombro
force-pushed
the
add_integration_tests
branch
8 times, most recently
from
e7a7579 to
f4e3b6f
Compare
eskombro
force-pushed
the
add_integration_tests
branch
from
f4e3b6f to
2278605
Compare
eskombro
changed the title
eskombro
changed the title
Co-authored-by: Clémentine Urquizar <[email protected]>
Co-authored-by: Clémentine Urquizar <[email protected]>
| * @param intervalInMs number of milliseconds before requesting the status again | ||
| * @throws Exception if timeout is reached | ||
| */ | ||
| public void waitForPendingUpdate(int updateId, int timeoutInMs, int intervalInMs) throws Exception { |
There was a problem hiding this comment.
maybe something like a CompletableFuture<UpdateStatus> would be better for something like this.
Supply the updateId -> thenApply(...) the while loop -> call .get(timeout)
| public String moviesFileToString () { | ||
| String content = ""; | ||
| String filePath = "src/test/java/com/meilisearch/sdk/movies.json"; | ||
| try | ||
| { | ||
| content = new String ( Files.readAllBytes( Paths.get(filePath) ) ); | ||
| } | ||
| catch (IOException e) | ||
| { | ||
| e.printStackTrace(); | ||
| } | ||
| this.movies_data = content; | ||
| return content; | ||
| } |
There was a problem hiding this comment.
please put the json file unter src/test/resources/movies.json
src/test/java should only contain java files
Also, please load the File from classpath.
This way we can move around our source files without to worry about where the json is located ;)
| String movies_data; | ||
| Movie[] movies = new Movie[10]; | ||
| Gson gson = new Gson(); |
There was a problem hiding this comment.
more of a general point: unless you really need fields package private, just declare them as private.
helps to avoid other parts of the program fucking around with the internal state of the class.
|
@niemannd thanks for your review! |
The goal for this PR is:
waitForPendingUpdate()method (Add WaitForPendingUpdate method #23)(it will be mandatory to pass integration tests for merging a PR into master after this PR is merged)
Related to this issue: @niemannd propose some unit testing in #31
Closes #14
Closes #23