-
Notifications
You must be signed in to change notification settings - Fork 35
Do not set environment variables in tests #2008
Conversation
We have a number of intermittently failing tests that are due to setting environment variables within the tests.
Rust tests run in parallel and the environment is basically a singleton. Therefore, these tests leak state with each other and are non-deterministic.
In addition, we are pulling items between action plugins, where we could factor it out into the env instead.
Fixes #1826.
Signed-off-by: Joe Grund <jgrund@whamcloud.io>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The root of the problem is that one plugin needs to know about another.
@@ -125,7 +126,9 @@ mod lpurge_conf_tests { | |||
freelo: 60, | |||
mailbox: "foobar".to_string(), | |||
}; | |||
let file = conf_name(&cfg).await.expect("name could not be created"); | |||
let file = conf_name(&cfg, "/etc/lpurge/{fs}/{ost}-{pool}.conf") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
write!( | ||
f, | ||
impl Config { | ||
fn generate_unit(&self, socket: String) -> String { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this could be unified with expand_path_fmt
: we just format strings, file name of content.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you expand on how the unification would work? One item is generating a systemd unit the other is generating a path with optional values.
Are you suggesting to use strfmt for both fns?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, strfmt for both. Even maybe in one pass, like cfg.write(socket, file_name_template);
There are some unused imports. |
Signed-off-by: Joe Grund <jgrund@whamcloud.io>
* Do not set environment variables in tests
We have a number of intermittently failing tests that are due to setting environment variables within the tests.
Rust tests run in parallel and the environment is basically a singleton. Therefore, these tests leak state with each other and are non-deterministic.
In addition, we are pulling items between action plugins, where we could factor it out into the env instead.
Fixes #1826.
Signed-off-by: Joe Grund <jgrund@whamcloud.io>
* Remove unused imports
Signed-off-by: Joe Grund <jgrund@whamcloud.io>
We have a number of intermittently failing tests that are due to setting environment variables within the tests.
Rust tests run in parallel and the environment is basically a singleton. Therefore, these tests leak state with each other and are non-deterministic.
In addition, we are pulling items between action plugins, where we could factor it out into the env instead.
Fixes #1826.
Signed-off-by: Joe Grund jgrund@whamcloud.io
This change is![Reviewable](https://camo.githubusercontent.com/23b05f5fb48215c989e92cc44cf6512512d083132bd3daf689867c8d9d386888/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)