Add option to query hosts via regex #77
Conversation
This allows various host interactions via regex matches to reduce the effort to induce more complex interactions. While this has changed the explicit API contract in some cases, this should not affect most test cases as the `ToIpAddrs` trait implements `ToIpAddr` for all `T` that implement `ToIpAddr`.
| @@ -10,6 +11,10 @@ pub trait ToIpAddr { | |||
| fn to_ip_addr(&self, dns: &mut Dns) -> IpAddr; | |||
| } | |||
|
|
|||
| pub trait ToIpAddrs { | |||
There was a problem hiding this comment.
If we break the contract and need to bump the version anyways, should we consider consolidating into a single trait?
There was a problem hiding this comment.
We could do, however there is the need for a trait like ToIpAddr for the sim#client and sim#host functions. As in these cases, the trait doubles as a generator rather than only a matcher.
| impl ToIpAddrs for Regex { | ||
| fn to_ip_addrs(&self, dns: &mut Dns) -> Vec<IpAddr> { | ||
| #[allow(clippy::needless_collect)] | ||
| let hosts = dns.names.keys().cloned().collect::<Vec<_>>(); |
There was a problem hiding this comment.
Yes, I battled with this one for quite a while. There's an open issue for clippy not to produce a false positive in this situation: rust-lang/rust-clippy#6066
| /// Lookup an IP address by host name. Use regex to match a number of hosts. | ||
| /// | ||
| /// Must be called from within a Turmoil simulation. | ||
| pub fn lookup_many(addr: impl ToIpAddrs) -> Vec<IpAddr> { |
There was a problem hiding this comment.
Seems only a Regex would be useful here, so perhaps we can just be explicit instead of using the trait?
There was a problem hiding this comment.
The ToIpAddrs trait could also support a Vec of string matches (actually not with this revision, but I'll update it), so for example another valid input would be:
lookup_many(vec!["host-a", "host-b"]);I was also looking into passing predicate callbacks, but I ran into trouble with conflicting generics with the ToIpAddr impl for ToIpAddrs.
Changing this, however, will affect all the other methods that currently take ToIpAddr (hold, release, partition). It may be a nice convenience if we asserted that the lookup_many should produce at least one element in each set, as its use could otherwise produce subtle bugs.
There was a problem hiding this comment.
It may be a nice convenience if we asserted that the lookup_many should produce at least one element in each set, as its use could otherwise produce subtle bugs.
Does this mean that if we look up two hosts we must see at least two hosts returning?
Updates doc comments, improves the tests and rewrites the Vec implementation to support any `T` that implements `ToIpAddr`.
This adds a separate optional regex flag to ensure that non-std features are optional.
This type now simply exists to shadow all existing `ToIpAddr` implementers and add the regex option when the feature flag is enabled.
| @@ -78,7 +78,7 @@ impl Rt { | |||
| let (tokio, local) = init(); | |||
|
|
|||
| _ = mem::replace(&mut self.tokio, tokio); | |||
| _ = mem::replace(&mut self.local, local); | |||
| drop(mem::replace(&mut self.local, local)); | |||
There was a problem hiding this comment.
Yes, it either wants you to explicitly drop or await the return here.
This allows various host interactions via regex matches to reduce the effort to induce more complex interactions. While this has changed the explicit API contract in some cases, this should not affect most test cases as the
ToIpAddrstrait implementsToIpAddrfor allTthat implementToIpAddr.