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.

Sign up for GitHub

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

Jump to bottom

Refactor ExponentialBackoffManager Tests to Remove Thread.sleep(). #495

Merged
merged 1 commit into from
Oct 24, 2023
Merged

Refactor ExponentialBackoffManager Tests to Remove Thread.sleep(). #495

merged 1 commit into from
Oct 24, 2023

Conversation

arturobernalg
Copy link
Member

This PR refactors the unit tests for ExponentialBackoffManager to remove the use of Thread.sleep(), making the tests more reliable and faster.

@ok2c
Copy link
Member

ok2c commented Oct 21, 2023

@arturobernalg

I just saw this error with the latest code in one of my CI runs. Could you please take a look at it as well?

[INFO] Running org.apache.hc.client5.http.impl.classic.TestLinearBackoffManager
Error:  Tests run: 8, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.008 s <<< FAILURE! - in org.apache.hc.client5.http.impl.classic.TestLinearBackoffManager
Error:  org.apache.hc.client5.http.impl.classic.TestLinearBackoffManager.backoffDoesNotAdjustDuringCoolDownPeriod  Time elapsed: 0.002 s  <<< FAILURE!
org.opentest4j.AssertionFailedError: expected: <5> but was: <6>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:166)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:161)
	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:629)
	at org.apache.hc.client5.http.impl.classic.TestLinearBackoffManager.backoffDoesNotAdjustDuringCoolDownPeriod(TestLinearBackoffManager.java:88)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)

@arturobernalg arturobernalg force-pushed the exponentialBackoffFixTest branch 2 times, most recently from d45d68d to aa7807e Compare October 21, 2023 16:47
@arturobernalg
Copy link
Member Author

@arturobernalg

I just saw this error with the latest code in one of my CI runs. Could you please take a look at it as well?

[INFO] Running org.apache.hc.client5.http.impl.classic.TestLinearBackoffManager
Error:  Tests run: 8, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.008 s <<< FAILURE! - in org.apache.hc.client5.http.impl.classic.TestLinearBackoffManager
Error:  org.apache.hc.client5.http.impl.classic.TestLinearBackoffManager.backoffDoesNotAdjustDuringCoolDownPeriod  Time elapsed: 0.002 s  <<< FAILURE!
org.opentest4j.AssertionFailedError: expected: <5> but was: <6>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:166)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:161)
	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:629)
	at org.apache.hc.client5.http.impl.classic.TestLinearBackoffManager.backoffDoesNotAdjustDuringCoolDownPeriod(TestLinearBackoffManager.java:88)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)

HI @ok2c
I've adjusted the backoffDoesNotAdjustDuringCoolDownPeriod test to better simulate the cool-down period by manipulating the lastRouteBackoffs map.

TY

@ok2c
Copy link
Member

ok2c commented Oct 22, 2023

@arturobernalg What about TestLinearBackoffManager?

@arturobernalg arturobernalg force-pushed the exponentialBackoffFixTest branch 3 times, most recently from 6bcd8b4 to 5d40de1 Compare October 23, 2023 19:31
@arturobernalg
Copy link
Member Author

TestLinearBackoffManager

@ok2c
Was modify a couple day before 5c61b0a but i didn't have updated my branch.

@ok2c
Copy link
Member

ok2c commented Oct 24, 2023

TestLinearBackoffManager

@ok2c Was modify a couple day before 5c61b0a but i didn't have updated my branch.

@arturobernalg I am afraid the test case still fails intermittently with your latest changes. We can leave it for now and see how often the failure occurs (if at all)

@arturobernalg
Refactor ExponentialBackoffManager and TestLinearBackoffManager Tests…
3428dc4 
… to Remove Thread.sleep().

This commit enhances the ExponentialBackoffManager and TestLinearBackoffManager unit tests by replacing the use of Thread.sleep() with direct manipulation of internal state to simulate the cooldown period. This change improves test reliability and ensures consistent behavior in resource-constrained environments.
@arturobernalg arturobernalg force-pushed the exponentialBackoffFixTest branch from 5d40de1 to 3428dc4 Compare October 24, 2023 16:51
@arturobernalg arturobernalg merged commit f9fd30e into apache:5.4.x Oct 24, 2023
7 checks passed
ok2c pushed a commit that referenced this pull request Dec 6, 2023
@arturobernalg @ok2c
Refactor ExponentialBackoffManager and TestLinearBackoffManager Tests…
5ab7f21 
… to Remove Thread.sleep(). (#495)

This commit enhances the ExponentialBackoffManager and TestLinearBackoffManager unit tests by replacing the use of Thread.sleep() with direct manipulation of internal state to simulate the cooldown period. This change improves test reliability and ensures consistent behavior in resource-constrained environments.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ok2c ok2c ok2c approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@arturobernalg @ok2c