Fix unit test testFailsHealthOnHungIOBeyondHealthyTimeout() by incresing the max waiting time before assertion

[tlfeng]

Description

Try to fix the unit test testFailsHealthOnHungIOBeyondHealthyTimeout() in class FsHealthServiceTests, according to the idea in comment #1567 (comment)

• Increase the max waiting time between cancelling the IO hanging and checking the File System health status. Increase the 2x multiplier to 3x that applied to refreshInterval.
• Remove the duplicate assertion statements for getting better error message.

Note:
1
The test failure can not be reproduced easily locally, and only sometime shows up in the CI workflow. I guess the reason of the test failure is there are dozens of file directories have to be restoring from the IO hanging status in the below step,

OpenSearch/server/src/test/java/org/opensearch/monitor/fs/FsHealthServiceTests.java

Lines 237 to 238 in 538b40a

	logger.info("--> Fix file system disruption");
	disruptFileSystemProvider.injectIODelay.set(false);

but the max waiting time in below code is not enough for all those file directories restored to a healthy state.

OpenSearch/server/src/test/java/org/opensearch/monitor/fs/FsHealthServiceTests.java

Line 242 in 538b40a

delayBetweenChecks + (2 * refreshInterval),

2
Before removing the duplicate assertion statement:

org.opensearch.monitor.fs.FsHealthServiceTests > testFailsHealthOnHungIOBeyondHealthyTimeout FAILED
    java.lang.AssertionError
        at __randomizedtesting.SeedInfo.seed([20C97377870AC21B:A2BD2D7EF3F6B453]:0)
        at org.junit.Assert.fail(Assert.java:86)
        at org.junit.Assert.assertTrue(Assert.java:41)
        at org.junit.Assert.assertTrue(Assert.java:52)
        at org.opensearch.monitor.fs.FsHealthServiceTests.testFailsHealthOnHungIOBeyondHealthyTimeout(FsHealthServiceTests.java:239)

After removing, the error message can be more intuitive:

org.opensearch.monitor.fs.FsHealthServiceTests > testFailsHealthOnHungIOBeyondHealthyTimeout FAILED
    java.lang.AssertionError: expected:<HEALTHY> but was:<UNHEALTHY>
        at __randomizedtesting.SeedInfo.seed([8E47558C149873A4:C330B85606405EC]:0)
        at org.junit.Assert.fail(Assert.java:88)
        at org.junit.Assert.failNotEquals(Assert.java:834)
        at org.junit.Assert.assertEquals(Assert.java:118)
        at org.junit.Assert.assertEquals(Assert.java:144)
        at org.opensearch.monitor.fs.FsHealthServiceTests.testFailsHealthOnHungIOBeyondHealthyTimeout(FsHealthServiceTests.java:243)

Issues Resolved

#1567 - I wish it could be resolved after the PR.
#1307 and #1450 - These 2 issues reporting the same test failure as #1567.

Check List

• New functionality includes testing.
  • All tests pass
• New functionality has been documented.
  • New functionality has javadoc added
• Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[opensearch-ci-bot]

Can one of the admins verify this patch?

[opensearch-ci-bot]

✅   Gradle Check success fbaea49
Log 1419

Reports 1419

[tlfeng]

Hi @Bukhtawar, I would like to hear your opinion on this change. 😁 Thank you.

[dblock]

I'm going to merge it since it visibly reduces the amount of test failures.

[dblock]

This failed to backport automatically in https://github.com/opensearch-project/OpenSearch/runs/4486636906?check_suite_focus=true and will need to be done by hand, please, cc: @tlfeng.

[tlfeng]

Hi @dblock, I realized that the PR #1167 that introduced the test testFailsHealthOnHungIOBeyondHealthyTimeout() isn't merged to 1.x branch. (https://github.com/opensearch-project/OpenSearch/blob/1.x/server/src/test/java/org/opensearch/monitor/fs/FsHealthServiceTests.java)
Although the backport is not needed, I will keep an eye when the unit test being backported.

[tlfeng]

Note: Pending backport along with c05e0d9, after the PR #1269 merged.

[Bukhtawar]

@tlfeng can you confirm if the test fixes have been merged to 1.x? I tried pulling in these changes as well

[tlfeng]

@tlfeng can you confirm if the test fixes have been merged to 1.x? I tried pulling in these changes as well

Hi @Bukhtawar, thanks a lot for pulling the test fix into your PR #1269! The unit test changes in your commit 88d1d65 are totally correct with commit baa10b9 and c05e0d9.