Refactor integration test core into a new 'obs-commander-tests' crate #60
Conversation
This file contains hidden or 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
refi64
force-pushed
the
wip/refi64/commander-tests
branch
from
2fa48b7 to
4ced142
Compare
|
@refi64 as a first pass can you fix CI and the conflicts? |
refi64
force-pushed
the
wip/refi64/commander-tests
branch
from
4ced142 to
cb034e3
Compare
|
@sjoerdsimons conflicts fixed, I don't think |
refi64
force-pushed
the
wip/refi64/commander-tests
branch
2 times, most recently
from
fe73a54 to
fb9b128
Compare
refi64
force-pushed
the
wip/refi64/commander-tests
branch
from
fb9b128 to
44c397f
Compare
refi64
force-pushed
the
wip/refi64/commander-tests
branch
2 times, most recently
from
2e43c59 to
0a098a8
Compare
Passing around `self` risks confusing the borrow checker if we need to invoke a method on a field, and it's rather confusing to begin with.
All the generic command handling is now part of an `actions` module, and logging happens through a rather cursed magic bridge in the new `logging` module to remove the widespread use of outputln.
This is largely just a rename, as well as the slightly-silly `obs-commander-test-support` crate that exists to avoid making the primary `obs-commander` crate depend on testing dependencies.
Most of this logic is rather intricate and shareable between multiple implementations, so it makes sense for them to be in a separate crate. This necessitates quite a bit of refactoring of the test structure so that the shared test code can actually run commands and inspect the logs + artifacts. Since there are various somewhat strange requirements around command execution (i.e. needing to use timeouts but still get the result), that API was redone around a builder trait, which also makes it easy for implementation-specific tests to have their own methods on it.
refi64
force-pushed
the
wip/refi64/commander-tests
branch
from
0a098a8 to
0f81984
Compare
| resolver = "3" | ||
| members = [ | ||
| "obs-commander", | ||
| "obs-commander-tests", |
There was a problem hiding this comment.
ooi why both tests and test-support?
|
|
||
| pub const MONITOR_TEST_OLD_STATUS_SLEEP_DURATION: Duration = Duration::from_millis(100); | ||
|
|
||
| pub const MONITOR_TEST_LOG_TAIL: u64 = 50; |
There was a problem hiding this comment.
newline here but not between the other consts?
| let srcmd5_prefix = format!( | ||
| "srcmd5 '{}' ", | ||
| if log_test == MonitorLogTest::Unavailable { | ||
| ZERO_REV_SRCMD5.to_owned() |
There was a problem hiding this comment.
nitpick; why converting to an owned value if it's being formatting anyway?
Comment on lines
+553
to
+554
| let prune = context.run().command("prune").go().await; | ||
| assert!(!prune.ok()); |
There was a problem hiding this comment.
if you use unwrap_err() instead when it fails it will dump what Ok value got returned. If that's not useful at least assert on is_err()`
There was a problem hiding this comment.
(same pattern in other places)
| pub const EXECUTION_DEFAULT_TIMEOUT: Duration = Duration::from_secs(30); | ||
|
|
||
| #[async_trait] | ||
| pub trait RunBuilder<'context>: Send + Sync + Sized { |
There was a problem hiding this comment.
Would really help if these traits had documentation wrt. to what their meant to do (both overall and the functions)
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.
As usual, only the last commit is new, depends on #58.