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 platform specific assumptions in OS & Process probes tests #12929

Closed
wants to merge 0 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@tlrx
Member

tlrx commented Aug 17, 2015

This pull request changes the tests assumptions in OSProbeTests and ProcessProbeTests in order to be more platform specific (at least for Windows) when checking the stats results.

There is good chance that some tests fail on some platforms/JVM but it's a good thing since it will help us to know were stats are available.

It also fixed the test ̀ProcessProbeTests.testProcessStats` that failed on CI with the following error:

  > Expected: a value equal to or greater than <0s>
   >      but: <-1s> was less than <0s>
   >    at __randomizedtesting.SeedInfo.seed([2C939BD6917275D4:72B58150CBA0DEB7]:0)
   >    at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
   >    at org.elasticsearch.monitor.process.ProcessProbeTests.testProcessStats(ProcessProbeTests.java:49)

Because CPU percent can be negative if the system recent CPU usage is not available and the test checks that it was greater or equal to 0. This commit fix this.

@s1monw

This comment has been minimized.

Show comment
Hide comment
@s1monw

s1monw Aug 17, 2015

Contributor

LGTM thanks

Contributor

s1monw commented Aug 17, 2015

LGTM thanks

@s1monw

This comment has been minimized.

Show comment
Hide comment
@s1monw

s1monw Aug 17, 2015

Contributor

this can go into 2.0?

Contributor

s1monw commented Aug 17, 2015

this can go into 2.0?

@tlrx

This comment has been minimized.

Show comment
Hide comment
@tlrx

tlrx Aug 17, 2015

Member

@s1monw thanks for your review. Yes it can go in 2.0, we just have to know that some tests might fail and might require some test code adaptation.

Member

tlrx commented Aug 17, 2015

@s1monw thanks for your review. Yes it can go in 2.0, we just have to know that some tests might fail and might require some test code adaptation.

@tlrx tlrx closed this Aug 17, 2015

@tlrx tlrx added v2.0.0 and removed review labels Aug 17, 2015

@tlrx tlrx deleted the tlrx:process-stats-tests-sometimes-fails branch Aug 17, 2015

@clintongormley clintongormley added v2.0.0-beta1 and removed v2.0.0 labels Aug 18, 2015

@clintongormley clintongormley removed the v2.1.0 label Nov 22, 2015

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