Skip to content
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

WIP: concurrent integration tests #427

Closed
wants to merge 1 commit into from

Conversation

justinmoon
Copy link
Contributor

@justinmoon justinmoon commented Aug 19, 2022

This PR generates random ports for server and gateway to bind to. I pulled in a portpicker dependency. That project is just 75 LOC with public domain license, so I think we could just copy any relevant code into our project if this works.

@justinmoon
Copy link
Contributor Author

CI gets stuck on the runs_consensus_if_tx_submitted tests ... will investigate more later

@justinmoon
Copy link
Contributor Author

Eric mentioned alternative fix for making the gateway work in concurrent tests: don't bind to a port. "Interconnect" is able to fake the network layer for our client / server communications in tests. We could do something similar for the gateway.

@elsirion
Copy link
Contributor

We could probably re-use the great API @maan2003 built

#[async_trait]
pub trait TypedApiEndpoint {
type State: Sync;
/// example: /transaction
const PATH: &'static str;
type Param: serde::de::DeserializeOwned + Send;
type Response: serde::Serialize;
async fn handle(state: &Self::State, params: Self::Param) -> Result<Self::Response, ApiError>;
}
#[doc(hidden)]
pub mod __reexports {
pub use serde_json;
}
/// # Example
///
/// ```rust
/// # use fedimint_api::module::{api_endpoint, ApiEndpoint};
/// struct State;
///
/// let _: ApiEndpoint<State> = api_endpoint! {
/// "/foobar",
/// async |state: &State, params: ()| -> i32 {
/// Ok(0)
/// }
/// };
/// ```
#[macro_export]
macro_rules! __api_endpoint {
(
$path:expr,
async |$state:ident: &$state_ty:ty, $param:ident: $param_ty:ty| -> $resp_ty:ty $body:block
) => {{
struct Endpoint;
#[async_trait::async_trait]
impl $crate::module::TypedApiEndpoint for Endpoint {
const PATH: &'static str = $path;
type State = $state_ty;
type Param = $param_ty;
type Response = $resp_ty;
async fn handle(
$state: &Self::State,
$param: Self::Param,
) -> ::std::result::Result<Self::Response, $crate::module::ApiError> {
$body
}
}
ApiEndpoint {
path: <Endpoint as $crate::module::TypedApiEndpoint>::PATH,
handler: |m, param| {
Box::pin(async move {
let params = $crate::module::__reexports::serde_json::from_value(param)
.map_err(|e| $crate::module::ApiError::bad_request(e.to_string()))?;
let ret =
<Endpoint as $crate::module::TypedApiEndpoint>::handle(m, params).await?;
Ok($crate::module::__reexports::serde_json::to_value(ret)
.expect("encoding error"))
})
},
}
}};
}
pub use __api_endpoint as api_endpoint;
/// Definition of an API endpoint defined by a module `M`.
pub struct ApiEndpoint<M> {
/// Path under which the API endpoint can be reached. It should start with a `/`
/// e.g. `/transaction`. E.g. this API endpoint would be reachable under `/module_name/transaction`
/// depending on the module name returned by `[FedertionModule::api_base_name]`.
pub path: &'static str,
/// Handler for the API call that takes the following arguments:
/// * Reference to the module which defined it
/// * Request parameters parsed into JSON `[Value](serde_json::Value)`
pub handler:
for<'a> fn(&'a M, serde_json::Value) -> BoxFuture<'a, Result<serde_json::Value, ApiError>>,
}

@justinmoon
Copy link
Contributor Author

Closing this. #497 is a better approach.

@justinmoon justinmoon closed this Sep 7, 2022
@NicolaLS
Copy link
Contributor

NicolaLS commented Sep 7, 2022

@justinmoon we might still want to switch to portpicker later because we have no guarantee that 4000+((testi_peers*2 + 1)*i) are not used by some other process on the machine

@justinmoon
Copy link
Contributor Author

@NicolaLS Could you try to PR this again? @elsirion was mentioning that he's still getting port-conflicts locally ...

@elsirion
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants