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

Add timeout config to RetryTest and CircuitBreakerTimeoutTest #543

Merged
merged 2 commits into from May 18, 2020

Conversation

Azquelt
Copy link
Member

@Azquelt Azquelt commented May 15, 2020

While testing the latest RC, I saw a failure I hadn't seen before in RetryTest where the timing is fairly tight and the test wasn't using TCKConfig.

After fixing that, I had a look through to see if there were any more tests which should clearly be using TCKConfig but aren't and I found CircuitBreakerTimeoutTest so I've fixed that too.

Fixes #544

Signed-off-by: Andrew Rouse <anrouse@uk.ibm.com>
Signed-off-by: Andrew Rouse <anrouse@uk.ibm.com>
@Azquelt Azquelt force-pushed the retry-delay-timeout-config branch from b3b36fe to de616f5 Compare May 15, 2020 10:06
@Emily-Jiang
Copy link
Member

@Azquelt can you open an issue for this so that all PRs for different branches can be associated with it?

@@ -1,6 +1,6 @@
/*
*******************************************************************************
* Copyright (c) 2018 Contributors to the Eclipse Foundation
* Copyright (c) 2018-2020 Contributors to the Eclipse Foundation
Copy link
Member

Choose a reason for hiding this comment

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

It should be 2018, 2020

Copy link
Member Author

Choose a reason for hiding this comment

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

If this is wrong, then it's wrong almost everywhere.

If it needs fixed, we should probably do a mass change over the whole repo.

Copy link
Member Author

Choose a reason for hiding this comment

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

Raised #545 for this.

@@ -1,6 +1,6 @@
/*
*******************************************************************************
* Copyright (c) 2018 Contributors to the Eclipse Foundation
* Copyright (c) 2018-2020 Contributors to the Eclipse Foundation
Copy link
Member

Choose a reason for hiding this comment

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

2018, 2020

@@ -1,6 +1,6 @@
/*
*******************************************************************************
* Copyright (c) 2016-2017 Contributors to the Eclipse Foundation
* Copyright (c) 2016-2020 Contributors to the Eclipse Foundation
Copy link
Member

Choose a reason for hiding this comment

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

2016, 2020

Copy link
Contributor

@Ladicek Ladicek left a comment

Choose a reason for hiding this comment

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

Change itself looks good to me.

Do you plan to file the same PR against the 2.0.x and 2.1.x branches?

@Azquelt
Copy link
Member Author

Azquelt commented May 15, 2020

Do you plan to file the same PR against the 2.0.x and 2.1.x branches?

Yes, will raise those PRs shortly.

@Azquelt
Copy link
Member Author

Azquelt commented May 15, 2020

@Emily-Jiang If you're happy for me to address the copyright date formatting under #545, then I think this (and the two backport PRs #546 and #547) are ready to go.

@Emily-Jiang Emily-Jiang merged commit dec3513 into eclipse:master May 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Timeout config missing on RetryTest and CircuitBreakerTimeoutTest
3 participants