ncouture / MockSSH

Mock an SSH server and define all commands it supports (Python, Twisted)
Other
123 stars 23 forks source link

support commands with PIPE #18

Closed ruivapps closed 7 years ago

ruivapps commented 7 years ago

is that possible to support PIPE command in ArgumentValidatingCommand ?

for example "ls -l | grep abc"

diff --git a/MockSSH.py b/MockSSH.py
index 18af0bf..1eb4c47 100755
--- a/MockSSH.py
+++ b/MockSSH.py
@@ -118,7 +118,10 @@ class ArgumentValidatingCommand(SSHCommand):
         self.name = name
         self.success_callbacks = success_callbacks
         self.failure_callbacks = failure_callbacks
-        self.required_arguments = [name] + list(args)
+        if isinstance(args[0], list):
+            self.required_arguments = [[name] + list(x) for x in args]
+        else:
+            self.required_arguments = [name] + list(args)
         self.protocol = None  # set in __call__

     def __call__(self, protocol, *args):
@@ -126,7 +129,7 @@ class ArgumentValidatingCommand(SSHCommand):
         return self

     def start(self):
-        if not tuple(self.args) == tuple(self.required_arguments):
+        if not list(self.args) == self.required_arguments:
             [func(self) for func in self.failure_callbacks]
         else:
             [func(self) for func in self.success_callbacks]
ncouture commented 7 years ago

Thanks for taking the time to share your improvements on MockSSH @ruivapps .

Would you be able to write a test to support this feature?

Finally, would you be able to submit a pull request for inclusion? (see documentation for details)


It's worth noting that MockSSH's tests are not well implemented. Some depend on MockSSH's threadedServer which is a no-no for testing Twisted applications (see #5 ). That said, I would like to make sure new features are tested at least for functionality (end to end test, see the tests directory for examples).

ruivapps commented 7 years ago

Nicolas,

I am currently on vacation, once I back to office. (2 weeks) I will find some time to write the test and create PR for you to review.

Cheers!

-Rui

On Sat, Jun 10, 2017 at 08:00 Nicolas Couture notifications@github.com wrote:

Assigned #18 https://github.com/ncouture/MockSSH/issues/18 to @ruivapps https://github.com/ruivapps.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ncouture/MockSSH/issues/18#event-1118223487, or mute the thread https://github.com/notifications/unsubscribe-auth/AAuYPeuSKBl1BWby4Ec63Gsr0Nbiojl_ks5sCoVWgaJpZM4Nkmdw .

ruivapps commented 7 years ago

Nicolas,

Just back to office today, and I spent some time to get #17 with unittest done. however, I’m not allowed to push branch to your repo. would you like to add me to the repo so I can push and create pull request? or if you like me to send files directly to you, I’m fine with it too

Thanks

-Rui

On Jun 11, 2017, at 4:07 AM, Rui Li rui.vapps@gmail.com wrote:

Nicolas,

I am currently on vacation, once I back to office. (2 weeks) I will find some time to write the test and create PR for you to review.

Cheers!

-Rui

On Sat, Jun 10, 2017 at 08:00 Nicolas Couture <notifications@github.com mailto:notifications@github.com> wrote: Assigned #18 https://github.com/ncouture/MockSSH/issues/18 to @ruivapps https://github.com/ruivapps.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ncouture/MockSSH/issues/18#event-1118223487, or mute the thread https://github.com/notifications/unsubscribe-auth/AAuYPeuSKBl1BWby4Ec63Gsr0Nbiojl_ks5sCoVWgaJpZM4Nkmdw.

ruivapps commented 7 years ago

I think #18 is by mistake. I wrote the unittest for it, and I found out (sha f8388) works fine with pipe. will close this one