Closed tonyandrewmeyer closed 4 weeks ago
The PR description seems to suggest this isn't quite ready for review. Gave it a quick look and found a couple of things that need clarification.
Ah indeed, I missed that when moving a bunch to ready. Should be ok for review now, although there are some things to decide before it could actually be mergable.
We (Tony, Dima, me) briefly discussed in our daily sync today and agreed:
1) execs
should be a set (like other top-level types are now)
2) The prefix-matching in Harness.handle_exec
has been very useful, and we think we should replicate it here. The first argument of scenario.Exec
will be command_prefix
(not keyword-only). Often just saying execs={scenario.Exec(['ls'])}
or similar will be enough.
3) The stdin
field is an output field, so we need a way to get the Exec
output instance. We'd add a Container.get_exec()
method where the lookup argument was either the command_prefix
or the input exec instance -- same as other get_foo
methods at the top level.
@PietroPasotti are you good with how this has ended up?
Firstly, some simple renaming:
Container.exec_mocks
becomesContainer.execs
Container.service_status
becomesContainer.service_statuses
ExecOutput
becomesExec
A behaviour change that is a bugfix:
ExecError
should have the stdout/stderr ifwait_output
was used (it's not readable afterwards by the charm, although the Scenario code doesn't enforce that).Some more substantial changes:
ExecArgs
), via a new context attributeexec_history
, which is a (default) dict where the key is the container name and the value is a list of Pebble exec's that have been run.We could add more of the functionality that Harness has, but I think this is a solid subset (I've wanted to be able to get to the stdin in writing tests myself, and the simple mock matching seems handy). It should be easy enough to extend in the future without needing API changes, I think, since this now has both input and output. The key parts that are missing are properly supporting binary IO and the execution context.