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

[TEST] Moved wipe* methods, randomIndexTemplate & ensureEstimatedStats to TestCluster #5542

Closed
wants to merge 1 commit into from

Conversation

javanna
Copy link
Member

@javanna javanna commented Mar 26, 2014

This is the first to make it possible to have a different impl of TestCluster (e.g. based on an external cluster) that has the same methods but a different impl for them (e.g. it might use the REST API to do the same instead of the Java API)

@javanna javanna self-assigned this Mar 26, 2014
@javanna javanna added the test label Mar 26, 2014
@@ -192,7 +169,7 @@
private static final Map<Class<?>, TestCluster> clusters = new IdentityHashMap<Class<?>, TestCluster>();

@BeforeClass
public final static void beforeClass() throws Exception {
public static void beforeClass() throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this not final anymore?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a warning fixed. Does it make sense to make a static method final? Could have been a separate change though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no worries I was just curious.

@s1monw
Copy link
Contributor

s1monw commented Mar 26, 2014

I left some comments but this LGTM in general

@javanna
Copy link
Member Author

javanna commented Mar 26, 2014

I just rebased and applied changes according to the review.

wipeIndices("_all"); // wipe after to make sure we fail in the test that didn't ack the delete
wipeTemplates();
wipeRepositories();
cluster().clear(); // wipe after to make sure we fail in the test that didn't ack the delete
ensureAllSearchersClosed();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be cluster.assertState() or something like this?

@s1monw
Copy link
Contributor

s1monw commented Mar 26, 2014

one small comments otherwise LGTM

@javanna
Copy link
Member Author

javanna commented Mar 26, 2014

Moved assertAllSearchersClosed and assertAllFilesClosed to ElasticsearchAssertions. They are both called from both ElasticsearchTestCase#beforeClass and ElasticsearchIntegrationTest#after (through TestCluster#assertAfterTest).

@s1monw
Copy link
Contributor

s1monw commented Mar 26, 2014

LGTM

…s from ElasticsearchIntegrationTest to TestCluster

This is the first to make it possible to have a different impl of TestCluster (e.g. based on an external cluster) that has the same methods but a different impl for them (e.g. it might use the REST API to do the same instead of the Java API)

Closes elastic#5542
@javanna javanna closed this in 89dd722 Mar 26, 2014
@javanna javanna added v2.0.0 and removed v1.1.1 labels Mar 26, 2014
javanna added a commit that referenced this pull request Mar 26, 2014
…s from ElasticsearchIntegrationTest to TestCluster

This is the first to make it possible to have a different impl of TestCluster (e.g. based on an external cluster) that has the same methods but a different impl for them (e.g. it might use the REST API to do the same instead of the Java API)

Closes #5542
javanna added a commit that referenced this pull request Mar 26, 2014
…s from ElasticsearchIntegrationTest to TestCluster

This is the first to make it possible to have a different impl of TestCluster (e.g. based on an external cluster) that has the same methods but a different impl for them (e.g. it might use the REST API to do the same instead of the Java API)

Closes #5542
javanna added a commit that referenced this pull request Mar 26, 2014
…s from ElasticsearchIntegrationTest to TestCluster

This is the first to make it possible to have a different impl of TestCluster (e.g. based on an external cluster) that has the same methods but a different impl for them (e.g. it might use the REST API to do the same instead of the Java API)

Closes #5542
javanna added a commit that referenced this pull request Mar 26, 2014
…s from ElasticsearchIntegrationTest to TestCluster

This is the first to make it possible to have a different impl of TestCluster (e.g. based on an external cluster) that has the same methods but a different impl for them (e.g. it might use the REST API to do the same instead of the Java API)

Closes #5542
mute pushed a commit to mute/elasticsearch that referenced this pull request Jul 29, 2015
…s from ElasticsearchIntegrationTest to TestCluster

This is the first to make it possible to have a different impl of TestCluster (e.g. based on an external cluster) that has the same methods but a different impl for them (e.g. it might use the REST API to do the same instead of the Java API)

Closes elastic#5542
mute pushed a commit to mute/elasticsearch that referenced this pull request Jul 29, 2015
…s from ElasticsearchIntegrationTest to TestCluster

This is the first to make it possible to have a different impl of TestCluster (e.g. based on an external cluster) that has the same methods but a different impl for them (e.g. it might use the REST API to do the same instead of the Java API)

Closes elastic#5542
mute pushed a commit to mute/elasticsearch that referenced this pull request Jul 29, 2015
…s from ElasticsearchIntegrationTest to TestCluster

This is the first to make it possible to have a different impl of TestCluster (e.g. based on an external cluster) that has the same methods but a different impl for them (e.g. it might use the REST API to do the same instead of the Java API)

Closes elastic#5542
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>test Issues or PRs that are addressing/adding tests v0.90.14 v1.0.3 v1.1.1 v1.2.0 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants