chore: improve test coverage in cli config #2313
Conversation
github-actions
bot
added
the
type: chore
laststylebender14
changed the title
| ) -> Self { | ||
| Self { merge_type, consolidate_url, tree_shake, use_better_names } | ||
| } | ||
| } |
There was a problem hiding this comment.
Not sure if I will ever use this API if the default operator exists. It more comfortable to use default().merge_type(1).consolidate_url(0.5) etc. Than using Preset::new(1, 0.5) and then have to forcefully pass the others parameters. Also it's easy to make a mistake of swapping the intended values of merge_type with consolidate_url since both are f32.
There was a problem hiding this comment.
yeah, agreed changed to use setters.
src/core/resource_reader.rs
Outdated
| let original_url: Url = "https://tailcall.run".parse().unwrap(); | ||
| let original_request = reqwest::Request::new(reqwest::Method::GET, original_url.clone()); | ||
| let resource: Resource = original_request.try_clone().unwrap().into(); | ||
| if let Resource::Request(converted_req) = resource { |
There was a problem hiding this comment.
No if conditions in tests.
There was a problem hiding this comment.
umm, if we want to verify the variants of enums then we can either use match and if.
there's another way to verify this via external crate assert_matches!. will something like following work?
assert_matches!(
resource,
Resource::Request(req) if {
req.url().as_str() == original_url.as_str() &&
req.method() == Method::GET
}
);
tusharmath
merged commit 0d822ac
into
feat/add-support-for-headers-in-request
Summary:
Briefly describe the changes made in this PR.
Issue Reference(s):
Fixes #... (Replace "..." with the issue number)
Build & Testing:
cargo testsuccessfully../lint.sh --mode=fixto fix all linting issues raised by./lint.sh --mode=check.Checklist:
<type>(<optional scope>): <title>