xushiyan commented on code in PR #124:
URL: https://github.com/apache/hudi-rs/pull/124#discussion_r1870472362
##########
crates/core/src/storage/mod.rs:
##########
@@ -293,10 +299,6 @@ mod tests {
result.is_err(),
"Should return error when no base path is invalid."
);
- assert!(result
- .unwrap_err()
- .to_string()
- .contains("Failed to create storage"));
Review Comment:
should check error type instead of removing the test case
##########
crates/core/src/config/read.rs:
##########
@@ -74,11 +79,16 @@ impl ConfigParser for HudiReadConfig {
let get_result = configs
.get(self.as_ref())
.map(|v| v.as_str())
- .ok_or(anyhow!("Config '{}' not found", self.as_ref()));
+ .ok_or(ConfNotFound(self.as_ref().to_string()));
Review Comment:
```suggestion
.ok_or(ConfigNotFound(self.as_ref().to_string()));
```
##########
crates/core/src/config/read.rs:
##########
@@ -103,7 +112,7 @@ mod tests {
fn parse_invalid_config_value() {
let options = HashMap::from([(InputPartitions.as_ref().to_string(),
"foo".to_string())]);
let value = InputPartitions.parse_value(&options);
- assert!(value.err().unwrap().is::<ParseIntError>());
+ assert!(value.is_err());
Review Comment:
we should not relax the test case, esp. when we have custom error type
--
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]
