search

containers / podman-compose

a script to run docker-compose.yml using podman
GNU General Public License v2.0
4.88k stars 470 forks source link

Allow run/exec -e with equals sign in value #860

Closed aripollak closed 4 months ago

aripollak commented 4 months ago

Fixes: #798

aripollak commented 4 months ago

Happy to add unit tests where appropriate, but I'm a little confused; is there a place that would make sense to add them now?

p12tic commented 4 months ago

On a second look a different approach is possible in this particular case.

compose_run() contains a lot of argument processing that could be moved to a separate function (basically everything here https://github.com/containers/podman-compose/blob/main/podman_compose.py#L2419C39-L2455C42). Then we could add unit tests for that function.

Same with compose_exec().

aripollak commented 4 months ago

@p12tic how does it look now? Specfying all the expected args in the passed argparse.Namespaces in the tests seemed a little duplicative, but that felt more appropriate for a unit test than calling into PodmanCompose._parse_args(). I think the interface of compose_run_update_container_from_args() would be more ideal if it wasn't changing the container dict in-place, but I'm not familiar enough with the code to go and change it too much.

p12tic commented 4 months ago

@aripollak I agree with the entirety of your comment.

The PR looks good, thanks!