Skip to content

Conversation

@tlrx
Copy link
Member

@tlrx tlrx commented Feb 4, 2019

This pull request reenables the test MonitoringIT.testMonitoringService and gives it a bit more time to start. This is curious that the test testMonitoringBulk passes and not this one, so I'm opening this pull request and I'll give it many CI builds to see if it fails a lot or not.

Closes #29880

@tlrx tlrx added >test Issues or PRs that are addressing/adding tests v7.0.0 :Data Management/Monitoring v6.7.0 labels Feb 4, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features

@tlrx
Copy link
Member Author

tlrx commented Feb 4, 2019

Test is green, let's run it again: @elasticmachine run elasticsearch-ci/2

2 similar comments
@tlrx
Copy link
Member Author

tlrx commented Feb 4, 2019

Test is green, let's run it again: @elasticmachine run elasticsearch-ci/2

@tlrx
Copy link
Member Author

tlrx commented Feb 5, 2019

Test is green, let's run it again: @elasticmachine run elasticsearch-ci/2

@tlrx
Copy link
Member Author

tlrx commented Feb 5, 2019

Test is red but this is unrelated to this change, let's run it again: @elasticmachine run elasticsearch-ci/2

1 similar comment
@tlrx
Copy link
Member Author

tlrx commented Feb 5, 2019

Test is red but this is unrelated to this change, let's run it again: @elasticmachine run elasticsearch-ci/2

@tlrx
Copy link
Member Author

tlrx commented Feb 5, 2019

Test is green, let's run it again: @elasticmachine run elasticsearch-ci/2

1 similar comment
@tlrx
Copy link
Member Author

tlrx commented Feb 5, 2019

Test is green, let's run it again: @elasticmachine run elasticsearch-ci/2

@tlrx
Copy link
Member Author

tlrx commented Feb 6, 2019

Given the facts that this pull request got many successful builds and that the other test in the same class does not fail, I think we can reenable the muted test and see how it goes.

Can anyone from the @elastic/es-core-features team review this please?

@jasontedor jasontedor added v8.0.0 and removed v7.0.0 labels Feb 6, 2019
assertThat(clusterState.remove("state_uuid"), notNullValue());
assertThat(clusterState.remove("cluster_uuid"), notNullValue());
assertThat(clusterState.remove("master_node"), notNullValue());
assertThat(clusterState.remove("nodes"), notNullValue());
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious to why we are removing these assertions ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's been added for Zen 2 but removed afterwards, but not noticed since the test was muted.

Copy link
Contributor

@jakelandis jakelandis left a comment

Choose a reason for hiding this comment

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

LGTM

@tlrx
Copy link
Member Author

tlrx commented Feb 8, 2019

@elasticmachine run elasticsearch-ci/1

@tlrx tlrx merged commit 1446abd into elastic:master Feb 11, 2019
@tlrx tlrx deleted the reenable-monitoring-it-service branch February 11, 2019 10:37
@tlrx
Copy link
Member Author

tlrx commented Feb 11, 2019

Thanks @jakelandis ! I merged this pull request and I'm going to let it run on CI/master for a few days before backporting the change to other branches.

@jpountz jpountz removed the v7.2.0 label Mar 5, 2019
@jpountz
Copy link
Contributor

jpountz commented Mar 5, 2019

Removing the v7.1.0 and backport pending labels after discussing with @tlrx.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Data Management/Monitoring >test Issues or PRs that are addressing/adding tests v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MonitoringIT testMonitoringService fails in CI

6 participants