Copilot commented on code in PR #580:
URL:
https://github.com/apache/arrow-rs-object-store/pull/580#discussion_r2624395092
##########
src/client/retry.rs:
##########
@@ -731,6 +734,17 @@ mod tests {
e.source().unwrap().to_string(),
"HTTP error: error sending request",
);
+ // the HttpError type needs to be directly accessible in the source
chain
+ let mut err: &dyn Error = &e;
+ let mut found = false;
+ while let Some(source) = err.source() {
+ err = source;
+ if let Some(err) = err.downcast_ref::<HttpError>() {
+ found = true;
+ assert_eq!(err.kind(), HttpErrorKind::Request);
Review Comment:
The test continues iterating through the source chain after finding and
verifying the HttpError. Consider adding a break statement after line 744 to
exit the loop once the HttpError has been found and verified, making the test
more efficient and the intent clearer.
```suggestion
assert_eq!(err.kind(), HttpErrorKind::Request);
break;
```
##########
src/client/retry.rs:
##########
@@ -731,6 +734,17 @@ mod tests {
e.source().unwrap().to_string(),
"HTTP error: error sending request",
);
+ // the HttpError type needs to be directly accessible in the source
chain
+ let mut err: &dyn Error = &e;
+ let mut found = false;
+ while let Some(source) = err.source() {
+ err = source;
+ if let Some(err) = err.downcast_ref::<HttpError>() {
+ found = true;
+ assert_eq!(err.kind(), HttpErrorKind::Request);
Review Comment:
The variable shadowing here (using 'err' for both the outer loop variable
and the downcast result) could be confusing. Consider using a different
variable name for the downcast result, such as 'http_err', to improve code
clarity and avoid potential confusion.
```suggestion
if let Some(http_err) = err.downcast_ref::<HttpError>() {
found = true;
assert_eq!(http_err.kind(), HttpErrorKind::Request);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
