Skip to content
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

Make Reindex cancellation tests more uniform #18498

Merged
merged 1 commit into from May 23, 2016

Conversation

Projects
None yet
3 participants
@tlrx
Copy link
Member

commented May 20, 2016

This pull request makes all the Reindex cancellation tests more uniform. They now all inherit from the same abstract class ReindexCancelTestCase and follow the same behaviour.

It also fixes a small bug where the Task status was not updated if the task was cancelled even if the operations were proceeded.

This relates to #18329 (comment)

@tlrx

This comment has been minimized.

Copy link
Member Author

commented May 20, 2016

@nik9000 Can you have a look? Thanks

@nik9000

View changes

modules/reindex/src/test/java/org/elasticsearch/index/reindex/DeleteByQueryCancelTests.java Outdated

private static final int MAX_DELETIONS = 10;
private static final CyclicBarrier barrier = new CyclicBarrier(2);
public class DeleteByQueryCancelTests extends ReindexCancelTestCase<DeleteByQueryRequestBuilder> {

This comment has been minimized.

Copy link
@nik9000

nik9000 May 20, 2016

Contributor

I'd just make one CancelTests or something and have it check all three actions.

@nik9000

This comment has been minimized.

Copy link
Contributor

commented May 20, 2016

I left a one comment about squashing the tests into a single file but otherwise LGTM.

@tlrx tlrx force-pushed the tlrx:change-reindex-cancel-tests branch to b0b5030 May 23, 2016

@tlrx tlrx merged commit b0b5030 into elastic:master May 23, 2016

1 check passed

CLA Commit author is a member of Elasticsearch
Details

@tlrx tlrx removed the review label May 23, 2016

@tlrx

This comment has been minimized.

Copy link
Member Author

commented May 23, 2016

Thanks @nik9000 ! I merged all 3 tests in a CancelTests class.

@tlrx tlrx deleted the tlrx:change-reindex-cancel-tests branch May 23, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.