Skip to content

Conversation

@rvolosatovs
Copy link
Member

@rvolosatovs rvolosatovs commented Feb 19, 2023

Feature or Problem

  • Replace the usage of free-form TomlMap by a structured, typed Config struct
  • Remove bin_path from config
  • Require passing of Config and path to test_provider at runtime

Related Issues

Closes #6

Release Information

Consumer Impact

Breaking changes:

  • load_config is replaced by Config::load method
  • test_provider requires path and Config as arguments
  • rust_backtrace config field is renamed to backtrace and its type is changed from string to a boolean

This is a breaking API change, downstream packages will have to be updated, e.g. for https://github.com/wasmCloud/capability-providers httpserver provider:

diff --git a/httpserver-rs/.cargo/config b/httpserver-rs/.cargo/config
new file mode 100644
index 0000000..dfa84e8
--- /dev/null
+++ b/httpserver-rs/.cargo/config
@@ -0,0 +1,2 @@
+[unstable]
+bindeps = true
diff --git a/httpserver-rs/provider_test_config.toml b/httpserver-rs/provider_test_config.toml
deleted file mode 100644
index 162254f..0000000
--- a/httpserver-rs/provider_test_config.toml
+++ /dev/null
@@ -1,28 +0,0 @@
-# configuration for httpserver test
-
-# name of compiled binary (usually project name unless overridden in [[bin]]
-# Required
-bin_path = "target/debug/httpserver"
-
-# set RUST_LOG environment variable (default "info")
-rust_log = "debug"
-
-# set RUST_BACKTRACE (default: 0)
-rust_backtrace = "1"
-
-# nats should be running. Uncomment to override the default url
-#nats_url = "0.0.0.0:4222"
-
-# redis should be running. Uncomment to override the default url
-#redis_url = "0.0.0.0:6379"
-
-# lattice prefix (default "default")
-#lattice_rpc_prefix = "default"
-
-# link name (default: "default")
-#link_name = "default"
-
-# name of contract under test
-contract_id = "wasmcloud:httpserver"
-
-values = { address = "0.0.0.0:9000" }
diff --git a/httpserver-rs/rust-toolchain.toml b/httpserver-rs/rust-toolchain.toml
new file mode 100644
index 0000000..5d56faf
--- /dev/null
+++ b/httpserver-rs/rust-toolchain.toml
@@ -0,0 +1,2 @@
+[toolchain]
+channel = "nightly"
diff --git a/httpserver-rs/tests/httpserver_test.rs b/httpserver-rs/tests/httpserver_test.rs
index c9d11d1..98e87aa 100644
--- a/httpserver-rs/tests/httpserver_test.rs
+++ b/httpserver-rs/tests/httpserver_test.rs
@@ -21,7 +21,7 @@ use wasmcloud_interface_httpserver::*;
 use wasmcloud_test_util::{
     check,
     cli::print_test_results,
-    provider_test::test_provider,
+    provider_test::{self, log, Config, LogLevel},
     testing::{TestOptions, TestResult},
 };
 #[allow(unused_imports)]
@@ -34,6 +34,20 @@ const SERVER_UNDER_TEST: &str = "http://localhost:9000";
 /// number of http requests in this test
 const NUM_RPC: u32 = 5;
 
+async fn test_provider() -> provider_test::Provider {
+    provider_test::test_provider(
+        env!("CARGO_BIN_EXE_httpserver"),
+        Config {
+            log_level: LogLevel(log::Level::Debug),
+            backtrace: true,
+            contract_id: "wasmcloud:httpserver".into(),
+            values: [("address".into(), "0.0.0.0:9000".into())].into(),
+            ..Default::default()
+        },
+    )
+    .await
+}
+
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
 async fn run_all() -> std::result::Result<(), Box<dyn std::error::Error>> {
     let opts = TestOptions::default();

Testing

Tested on platform(s)

  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
  • x86_64-windows

Platforms

Built on platform(s)

  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
  • x86_64-windows

Unit Test(s)

Acceptance or Integration

Manual Verification

wasmCloud/capability-providers#219 (wasmCloud/capability-providers@78fab3e in particular)

@rvolosatovs
Copy link
Member Author

rvolosatovs commented Feb 19, 2023

I'll undraft the PR once all downstream packages are adapted (and, hence, the design is validated)

rvolosatovs added a commit to rvolosatovs/capability-providers that referenced this pull request Mar 1, 2023
Add symlinks to workspace `target` until
wasmCloud/wasmcloud-test#22
is merged and integrated into this repository removing the need for
on-disk test configuration

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
rvolosatovs added a commit to rvolosatovs/capability-providers that referenced this pull request Mar 1, 2023
Add symlinks to workspace `target` until
wasmCloud/wasmcloud-test#22
is merged and integrated into this repository removing the need for
on-disk test configuration

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
rvolosatovs added a commit to rvolosatovs/capability-providers that referenced this pull request Mar 1, 2023
Add symlinks to workspace `target` until
wasmCloud/wasmcloud-test#22
is merged and integrated into this repository removing the need for
on-disk test configuration

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
rvolosatovs added a commit to rvolosatovs/capability-providers that referenced this pull request Mar 1, 2023
Add symlinks to workspace `target` until
wasmCloud/wasmcloud-test#22
is merged and integrated into this repository removing the need for
on-disk test configuration

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
@rvolosatovs rvolosatovs force-pushed the feat/runtime-config branch 2 times, most recently from 58a246f to de3e401 Compare March 1, 2023 15:37
@rvolosatovs rvolosatovs marked this pull request as ready for review March 1, 2023 15:40
@rvolosatovs rvolosatovs force-pushed the feat/runtime-config branch 2 times, most recently from ad14018 to bb101c2 Compare March 1, 2023 15:47
- Replace the usage of free-form `TomlMap` by a structured, typed `Config` struct
- Remove `bin_path` from config
- Require passing of `Config` and `path` to `test_provider` at runtime

Breaking changes:
- `load_config` is replaced by `Config::load` method
- `test_provider` requires `path` and `Config` as arguments
- `rust_backtrace` config field is renamed to `backtrace` and its type
  is changed from string to a boolean

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
@rvolosatovs rvolosatovs force-pushed the feat/runtime-config branch from bb101c2 to 791ca91 Compare March 2, 2023 14:56
Copy link
Contributor

@stevelr stevelr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like this - good improvement. Please make sure to rebase to main to address any merge conflicts. Also bump the version in Cargo.toml to 0.8.0.

/// URL at which NATS is accessible
pub nats_url: String,
/// URL at which Redis is accessible
pub redis_url: String,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Althoughredis_url was in the sample config (commented out), I don't think it should be part of the Config struct since most tests won't need it. If a test does need it, it fits better into the values map.

nats_url should be here because nearly ever test does need that

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.

test_provider should allow an optional hashmap of config options

3 participants