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: Allow global test cluster to have configurable settings source #7482

Conversation

Projects
None yet
5 participants
@spinscale
Copy link
Member

spinscale commented Aug 27, 2014

This allows to reuse the global test cluster with specific configurations,
which is useful in plugins.

Allows to change this in plugin tests like this:

public class FooTest extends ElasticsearchIntegrationTest {

    static {
        InternalTestCluster.DEFAULT_SETTINGS_SOURCE = new SettingsSource() { ... };
    }

    @Test
    public void foo() { ... }

@spinscale spinscale added the review label Aug 27, 2014

@s1monw

This comment has been minimized.

Copy link
Contributor

s1monw commented Aug 27, 2014

awesome maybe we can drop that extra ctor in InternalTestCluster too then?

@javanna

View changes

src/test/java/org/elasticsearch/test/InternalTestCluster.java Outdated
@@ -123,6 +123,8 @@

private final ESLogger logger = Loggers.getLogger(getClass());

public static SettingsSource CONFIGURABLE_SETTINGS_SOURCE = SettingsSource.EMPTY;

This comment has been minimized.

Copy link
@javanna

javanna Aug 27, 2014

Member

maybe package private instead of public?

@javanna

View changes

src/test/java/org/elasticsearch/test/SettingsSource.java Outdated
@@ -20,7 +20,7 @@

import org.elasticsearch.common.settings.Settings;

abstract class SettingsSource {
public abstract class SettingsSource {

This comment has been minimized.

Copy link
@javanna

javanna Aug 27, 2014

Member

same as above, if we make CONFIGURABLE_SETTINGS_SOURCE package private we might want to leave this package private as well.

@spinscale spinscale force-pushed the spinscale:enhancement/allow-configurable-settingssource-for-global-testcluster branch 4 times, most recently Aug 27, 2014

@spinscale

This comment has been minimized.

Copy link
Member Author

spinscale commented Aug 27, 2014

@s1monw I removed the simple unused constructor and left the other two in place. The TribeTests are using it as well and doesnt make too much sense for them to set it explicety in the tests.. this way everything can stay package private

@javanna

This comment has been minimized.

Copy link
Member

javanna commented Aug 27, 2014

Left a minor comment, other that that looks great to me!

@nik9000

This comment has been minimized.

Copy link
Contributor

nik9000 commented Aug 27, 2014

Sweet! I'm looking forward to this!

@javanna

This comment has been minimized.

Copy link
Member

javanna commented Aug 27, 2014

Hey @nik9000 right this would help #7040 too wouldn't it?

@nik9000

This comment has been minimized.

Copy link
Contributor

nik9000 commented Aug 27, 2014

@javanna yeah! It pretty much closes that issue.

@s1monw

This comment has been minimized.

Copy link
Contributor

s1monw commented Aug 27, 2014

LGTM

Test: Allow global test cluster to have configurable settings source
This allows to reuse the global test cluster with specific configurations,
which is useful in plugins.

@spinscale spinscale force-pushed the spinscale:enhancement/allow-configurable-settingssource-for-global-testcluster branch to 3aa72f2 Aug 27, 2014

@spinscale spinscale merged commit 3aa72f2 into elastic:master Aug 27, 2014

@spinscale spinscale added v2.0.0 and removed review labels Aug 27, 2014

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.