factors: Update runtime config and tests #2686
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Lann Martin <[email protected]>
lann
commented
Jul 26, 2024
Comment on lines
+55
to
+60
| type EnvFetcherFn = Box<dyn Fn(&str) -> Result<String, VarError> + Send + Sync>; | ||
|
|
||
| /// A config Provider that uses environment variables. | ||
| pub struct EnvVariablesProvider { | ||
| prefix: Option<String>, | ||
| env_fetcher: Box<dyn Fn(&str) -> Result<String, VarError> + Send + Sync>, | ||
| env_fetcher: EnvFetcherFn, |
rylev
approved these changes
Jul 29, 2024
| let mut env = Self::default(); | ||
| env.manifest.extend(manifest_merge); | ||
| env | ||
| pub fn extend_manifest(mut self, manifest_merge: toml::Table) -> Self { |
There was a problem hiding this comment.
This seems to be mimicking a builder pattern. Normally this would be a separate struct with a build method, but I think in this case, just mutating the TestEnvironment is fine. Just wanted to note it here as a place for potential future change.
| factors, | ||
| TomlRuntimeSource::new(&runtime_config, sqlite_config), | ||
| ) | ||
| let env = TestEnvironment::new(factors) |
There was a problem hiding this comment.
Suggested change
| let env = TestEnvironment::new(factors) | |
| let _ = TestEnvironment::new(factors) |
I'm surprised this doesn't lead to a warning?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I swear this started out as a focused commit...
Anyway: this is a random grab-bag of changes to simplify and homogenize factors runtime config and tests:
RuntimeFactors::configure_app#[derive(Default)]to derivedRuntimeFactors' runtime configs; replace()as an "empty source" withDefault::default()TestEnvironmentto use a builder-style interfacefactor_test.rs(commence bikeshedding!)WasiFactordeps