-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Stabilize most commonly failing tests #36956
Stabilize most commonly failing tests #36956
Conversation
API change check API changes are not detected in this pull request. |
if (proxyRetry) { | ||
// Exhausted retry attempt return an error. | ||
return Mono.error(new HttpProxyHandler.HttpProxyConnectException( | ||
"Failed to connect to proxy. Status: 407", responseAndHeaders.getT2())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious why we can't use response.getHeaders()
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This takes the Netty HttpHeaders
object, not the azure-core HttpHeaders
.
try { | ||
classPath = Class.forName(className).getCanonicalName(); | ||
return Class.forName(className).getCanonicalName(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: maybe call overload with initialize
flag set to false to minimize side-effects?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit confused on this comment, this is a utility method to help the String
-based constructor as this
has to be the first statement in a constructor.
// 23 characters that will be ASCII | ||
byte[] bytes = new byte[23]; | ||
|
||
// yyyy- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
year 10000
will be a tough one 🤣
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll be worried when I see that day 😄
Description
Fixes #36666
Attempts to stabilize the most commonly failing tests in the Core pipeline.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines