diff mbox series

[4/4] tests: test-hmp: print command execution result

Message ID 20171023151310.6462-5-vadim.galitsyn@profitbricks.com
State New
Headers show
Series [1/4] qmp: introduce 'writeconfig' command | expand

Commit Message

Vadim Galitsyn Oct. 23, 2017, 3:13 p.m. UTC
Provide HMP monitor command execution result as it would be seen
by user who established an HMP monitor session.

Currently many commands may silently fail without any sign of that.
This patch let this info to be printed once test is running in
verbose mode.

For the future it might be useful to fail the test if command has
failed, however it would require a bit of rework inside test
engine itself.

A simple example of silent failure without reporting it would to
add some non-existent HMP command into 'hmp_cmds' list. In this case
test will report it successfully passed without error.

Signed-off-by: Vadim Galitsyn <vadim.galitsyn@profitbricks.com>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: qemu-devel@nongnu.org
---
 tests/test-hmp.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Eduardo Otubo Oct. 25, 2017, 12:03 p.m. UTC | #1
On Mon, Oct 23, 2017 at 05:13:10PM +0200, Vadim Galitsyn wrote:
> Provide HMP monitor command execution result as it would be seen
> by user who established an HMP monitor session.
> 
> Currently many commands may silently fail without any sign of that.
> This patch let this info to be printed once test is running in
> verbose mode.
> 
> For the future it might be useful to fail the test if command has
> failed, however it would require a bit of rework inside test
> engine itself.
> 
> A simple example of silent failure without reporting it would to
> add some non-existent HMP command into 'hmp_cmds' list. In this case
> test will report it successfully passed without error.
> 
> Signed-off-by: Vadim Galitsyn <vadim.galitsyn@profitbricks.com>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Cc: qemu-devel@nongnu.org
> ---
>  tests/test-hmp.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/test-hmp.c b/tests/test-hmp.c
> index 8e21eee61c..1fd8e44eff 100644
> --- a/tests/test-hmp.c
> +++ b/tests/test-hmp.c
> @@ -79,10 +79,13 @@ static void test_commands(void)
>      int i;
>  
>      for (i = 0; hmp_cmds[i] != NULL; i++) {
> +        response = hmp("%s", hmp_cmds[i]);
>          if (verbose) {
> -            fprintf(stderr, "\t%s\n", hmp_cmds[i]);
> +            fprintf(stderr,
> +                    "\texecute HMP command: %s\n"
> +                    "\tresult             : %s\n",
> +                    hmp_cmds[i], response);
>          }
> -        response = hmp("%s", hmp_cmds[i]);
>          g_free(response);
>      }
>  
> -- 
> 2.13.1.394.g41dd433
> 
> 

Reviewed-by: Eduardo Otubo <otubo@redhat.com>
Dr. David Alan Gilbert Dec. 14, 2017, 11 a.m. UTC | #2
* Vadim Galitsyn (vadim.galitsyn@profitbricks.com) wrote:
> Provide HMP monitor command execution result as it would be seen
> by user who established an HMP monitor session.
> 
> Currently many commands may silently fail without any sign of that.
> This patch let this info to be printed once test is running in
> verbose mode.
> 
> For the future it might be useful to fail the test if command has
> failed, however it would require a bit of rework inside test
> engine itself.
> 
> A simple example of silent failure without reporting it would to
> add some non-existent HMP command into 'hmp_cmds' list. In this case
> test will report it successfully passed without error.
> 
> Signed-off-by: Vadim Galitsyn <vadim.galitsyn@profitbricks.com>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Cc: qemu-devel@nongnu.org

This test patch is useful without the rest of the series;

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Queued for HMP

Although I do also need to go and fix this tests idea of verbose to get
it from GTester rather than it's own env - I'll do that.

For the rest of the set, you need to convince Markus why
it's safe.

Dave


> ---
>  tests/test-hmp.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/test-hmp.c b/tests/test-hmp.c
> index 8e21eee61c..1fd8e44eff 100644
> --- a/tests/test-hmp.c
> +++ b/tests/test-hmp.c
> @@ -79,10 +79,13 @@ static void test_commands(void)
>      int i;
>  
>      for (i = 0; hmp_cmds[i] != NULL; i++) {
> +        response = hmp("%s", hmp_cmds[i]);
>          if (verbose) {
> -            fprintf(stderr, "\t%s\n", hmp_cmds[i]);
> +            fprintf(stderr,
> +                    "\texecute HMP command: %s\n"
> +                    "\tresult             : %s\n",
> +                    hmp_cmds[i], response);
>          }
> -        response = hmp("%s", hmp_cmds[i]);
>          g_free(response);
>      }
>  
> -- 
> 2.13.1.394.g41dd433
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff mbox series

Patch

diff --git a/tests/test-hmp.c b/tests/test-hmp.c
index 8e21eee61c..1fd8e44eff 100644
--- a/tests/test-hmp.c
+++ b/tests/test-hmp.c
@@ -79,10 +79,13 @@  static void test_commands(void)
     int i;
 
     for (i = 0; hmp_cmds[i] != NULL; i++) {
+        response = hmp("%s", hmp_cmds[i]);
         if (verbose) {
-            fprintf(stderr, "\t%s\n", hmp_cmds[i]);
+            fprintf(stderr,
+                    "\texecute HMP command: %s\n"
+                    "\tresult             : %s\n",
+                    hmp_cmds[i], response);
         }
-        response = hmp("%s", hmp_cmds[i]);
         g_free(response);
     }