We now test the deprecated commands everytime that we test the new
commands.  This makes unnecesary to add tests for deprecated commands.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 tests/migration-test.c | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index 799e24ebc6..51f49c74e9 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -369,7 +369,7 @@ static void migrate_check_parameter(QTestState *who, const char *parameter,
     QDECREF(rsp);
 }
 
-static void migrate_set_downtime(QTestState *who, const double value)
+static void deprecated_set_downtime(QTestState *who, const double value)
 {
     QDict *rsp;
     gchar *cmd;
@@ -388,7 +388,7 @@ static void migrate_set_downtime(QTestState *who, const double value)
     g_free(expected);
 }
 
-static void migrate_set_speed(QTestState *who, const char *value)
+static void deprecated_set_speed(QTestState *who, const char *value)
 {
     QDict *rsp;
     gchar *cmd;
@@ -402,6 +402,30 @@ static void migrate_set_speed(QTestState *who, const char *value)
     migrate_check_parameter(who, "max-bandwidth", value);
 }
 
+static void migrate_set_parameter(QTestState *who, const char *parameter,
+                                  const char *value)
+{
+    QDict *rsp;
+    gchar *cmd;
+
+    if (strcmp(parameter, "downtime-limit") == 0) {
+        deprecated_set_downtime(who, 0.12345);
+    }
+
+    if (strcmp(parameter, "max-bandwidth") == 0) {
+        deprecated_set_speed(who, "12345");
+    }
+
+    cmd = g_strdup_printf("{ 'execute': 'migrate-set-parameters',"
+                          "'arguments': { '%s': %s } }",
+                          parameter, value);
+    rsp = qtest_qmp(who, cmd);
+    g_free(cmd);
+    g_assert(qdict_haskey(rsp, "return"));
+    QDECREF(rsp);
+    migrate_check_parameter(who, parameter, value);
+}
+
 static void migrate_set_capability(QTestState *who, const char *capability,
                                    const char *value)
 {
@@ -530,8 +554,8 @@ static void test_migrate(void)
      * quickly, but that it doesn't complete precopy even on a slow
      * machine, so also set the downtime.
      */
-    migrate_set_speed(from, "100000000");
-    migrate_set_downtime(from, 0.001);
+    migrate_set_parameter(from, "max-bandwidth", "100000000");
+    migrate_set_parameter(from, "downtime-limit", "1");
 
     /* Wait for the first serial output from the source */
     wait_for_serial("src_serial");
-- 
2.14.3

On Fri, Dec 01, 2017 at 01:58:09PM +0100, Juan Quintela wrote:
> We now test the deprecated commands everytime that we test the new
> commands.  This makes unnecesary to add tests for deprecated commands.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  tests/migration-test.c | 32 ++++++++++++++++++++++++++++----
>  1 file changed, 28 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index 799e24ebc6..51f49c74e9 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -369,7 +369,7 @@ static void migrate_check_parameter(QTestState *who, const char *parameter,
>      QDECREF(rsp);
>  }
>  
> -static void migrate_set_downtime(QTestState *who, const double value)
> +static void deprecated_set_downtime(QTestState *who, const double value)
>  {
>      QDict *rsp;
>      gchar *cmd;
> @@ -388,7 +388,7 @@ static void migrate_set_downtime(QTestState *who, const double value)
>      g_free(expected);
>  }
>  
> -static void migrate_set_speed(QTestState *who, const char *value)
> +static void deprecated_set_speed(QTestState *who, const char *value)
>  {
>      QDict *rsp;
>      gchar *cmd;
> @@ -402,6 +402,30 @@ static void migrate_set_speed(QTestState *who, const char *value)
>      migrate_check_parameter(who, "max-bandwidth", value);
>  }
>  
> +static void migrate_set_parameter(QTestState *who, const char *parameter,
> +                                  const char *value)
> +{
> +    QDict *rsp;
> +    gchar *cmd;
> +
> +    if (strcmp(parameter, "downtime-limit") == 0) {
> +        deprecated_set_downtime(who, 0.12345);
> +    }
> +
> +    if (strcmp(parameter, "max-bandwidth") == 0) {
> +        deprecated_set_speed(who, "12345");
> +    }

I'm fine with current approach, but I would really prefer to put them
all into a standalone test, by just calling them one by one with some
specific numbers and that's all.

(luckily we only have two deprecated commands and limited tests,
 otherwise extra commands will be M*N, say "number of deprecated
 command" * "number of test mirations")

Thanks,

> +
> +    cmd = g_strdup_printf("{ 'execute': 'migrate-set-parameters',"
> +                          "'arguments': { '%s': %s } }",
> +                          parameter, value);
> +    rsp = qtest_qmp(who, cmd);
> +    g_free(cmd);
> +    g_assert(qdict_haskey(rsp, "return"));
> +    QDECREF(rsp);
> +    migrate_check_parameter(who, parameter, value);
> +}
> +
>  static void migrate_set_capability(QTestState *who, const char *capability,
>                                     const char *value)
>  {
> @@ -530,8 +554,8 @@ static void test_migrate(void)
>       * quickly, but that it doesn't complete precopy even on a slow
>       * machine, so also set the downtime.
>       */
> -    migrate_set_speed(from, "100000000");
> -    migrate_set_downtime(from, 0.001);
> +    migrate_set_parameter(from, "max-bandwidth", "100000000");
> +    migrate_set_parameter(from, "downtime-limit", "1");
>  
>      /* Wait for the first serial output from the source */
>      wait_for_serial("src_serial");
> -- 
> 2.14.3
> 

-- 
Peter Xu

Peter Xu <peterx@redhat.com> wrote:
> On Fri, Dec 01, 2017 at 01:58:09PM +0100, Juan Quintela wrote:
>> We now test the deprecated commands everytime that we test the new
>> commands.  This makes unnecesary to add tests for deprecated commands.
>> 
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  tests/migration-test.c | 32 ++++++++++++++++++++++++++++----
>>  1 file changed, 28 insertions(+), 4 deletions(-)
>> 
>> diff --git a/tests/migration-test.c b/tests/migration-test.c
>> index 799e24ebc6..51f49c74e9 100644
>> --- a/tests/migration-test.c
>> +++ b/tests/migration-test.c
>> @@ -369,7 +369,7 @@ static void migrate_check_parameter(QTestState *who, const char *parameter,
>>      QDECREF(rsp);
>>  }
>>  
>> -static void migrate_set_downtime(QTestState *who, const double value)
>> +static void deprecated_set_downtime(QTestState *who, const double value)
>>  {
>>      QDict *rsp;
>>      gchar *cmd;
>> @@ -388,7 +388,7 @@ static void migrate_set_downtime(QTestState *who, const double value)
>>      g_free(expected);
>>  }
>>  
>> -static void migrate_set_speed(QTestState *who, const char *value)
>> +static void deprecated_set_speed(QTestState *who, const char *value)
>>  {
>>      QDict *rsp;
>>      gchar *cmd;
>> @@ -402,6 +402,30 @@ static void migrate_set_speed(QTestState *who, const char *value)
>>      migrate_check_parameter(who, "max-bandwidth", value);
>>  }
>>  
>> +static void migrate_set_parameter(QTestState *who, const char *parameter,
>> +                                  const char *value)
>> +{
>> +    QDict *rsp;
>> +    gchar *cmd;
>> +
>> +    if (strcmp(parameter, "downtime-limit") == 0) {
>> +        deprecated_set_downtime(who, 0.12345);
>> +    }
>> +
>> +    if (strcmp(parameter, "max-bandwidth") == 0) {
>> +        deprecated_set_speed(who, "12345");
>> +    }
>
> I'm fine with current approach, but I would really prefer to put them
> all into a standalone test, by just calling them one by one with some
> specific numbers and that's all.

That means another test (at least), and we have, also at least three
deprecated comands:
- migrate_set_speed
- migrate_set_downtime
- migrate_set_cache_size

And each test makes things slower.  So I *thought* it would we wiser to
just check _always_ use the deprecated an the not deprecated one.

> (luckily we only have two deprecated commands and limited tests,
>  otherwise extra commands will be M*N, say "number of deprecated
>  command" * "number of test mirations")

Each test takes time, so adding tests make everything much slower.
Notice that setting a new setting is fast.

This was the way that I understood Dave he wanted.

Later, Juan.


>
> Thanks,
>
>> +
>> +    cmd = g_strdup_printf("{ 'execute': 'migrate-set-parameters',"
>> +                          "'arguments': { '%s': %s } }",
>> +                          parameter, value);
>> +    rsp = qtest_qmp(who, cmd);
>> +    g_free(cmd);
>> +    g_assert(qdict_haskey(rsp, "return"));
>> +    QDECREF(rsp);
>> +    migrate_check_parameter(who, parameter, value);
>> +}
>> +
>>  static void migrate_set_capability(QTestState *who, const char *capability,
>>                                     const char *value)
>>  {
>> @@ -530,8 +554,8 @@ static void test_migrate(void)
>>       * quickly, but that it doesn't complete precopy even on a slow
>>       * machine, so also set the downtime.
>>       */
>> -    migrate_set_speed(from, "100000000");
>> -    migrate_set_downtime(from, 0.001);
>> +    migrate_set_parameter(from, "max-bandwidth", "100000000");
>> +    migrate_set_parameter(from, "downtime-limit", "1");
>>  
>>      /* Wait for the first serial output from the source */
>>      wait_for_serial("src_serial");
>> -- 
>> 2.14.3
>> 

On Tue, Dec 26, 2017 at 08:51:10PM +0100, Juan Quintela wrote:
> Peter Xu <peterx@redhat.com> wrote:
> > On Fri, Dec 01, 2017 at 01:58:09PM +0100, Juan Quintela wrote:
> >> We now test the deprecated commands everytime that we test the new
> >> commands.  This makes unnecesary to add tests for deprecated commands.
> >> 
> >> Signed-off-by: Juan Quintela <quintela@redhat.com>
> >> ---
> >>  tests/migration-test.c | 32 ++++++++++++++++++++++++++++----
> >>  1 file changed, 28 insertions(+), 4 deletions(-)
> >> 
> >> diff --git a/tests/migration-test.c b/tests/migration-test.c
> >> index 799e24ebc6..51f49c74e9 100644
> >> --- a/tests/migration-test.c
> >> +++ b/tests/migration-test.c
> >> @@ -369,7 +369,7 @@ static void migrate_check_parameter(QTestState *who, const char *parameter,
> >>      QDECREF(rsp);
> >>  }
> >>  
> >> -static void migrate_set_downtime(QTestState *who, const double value)
> >> +static void deprecated_set_downtime(QTestState *who, const double value)
> >>  {
> >>      QDict *rsp;
> >>      gchar *cmd;
> >> @@ -388,7 +388,7 @@ static void migrate_set_downtime(QTestState *who, const double value)
> >>      g_free(expected);
> >>  }
> >>  
> >> -static void migrate_set_speed(QTestState *who, const char *value)
> >> +static void deprecated_set_speed(QTestState *who, const char *value)
> >>  {
> >>      QDict *rsp;
> >>      gchar *cmd;
> >> @@ -402,6 +402,30 @@ static void migrate_set_speed(QTestState *who, const char *value)
> >>      migrate_check_parameter(who, "max-bandwidth", value);
> >>  }
> >>  
> >> +static void migrate_set_parameter(QTestState *who, const char *parameter,
> >> +                                  const char *value)
> >> +{
> >> +    QDict *rsp;
> >> +    gchar *cmd;
> >> +
> >> +    if (strcmp(parameter, "downtime-limit") == 0) {
> >> +        deprecated_set_downtime(who, 0.12345);
> >> +    }
> >> +
> >> +    if (strcmp(parameter, "max-bandwidth") == 0) {
> >> +        deprecated_set_speed(who, "12345");
> >> +    }
> >
> > I'm fine with current approach, but I would really prefer to put them
> > all into a standalone test, by just calling them one by one with some
> > specific numbers and that's all.
> 
> That means another test (at least), and we have, also at least three
> deprecated comands:
> - migrate_set_speed
> - migrate_set_downtime
> - migrate_set_cache_size
> 
> And each test makes things slower.  So I *thought* it would we wiser to
> just check _always_ use the deprecated an the not deprecated one.
> 
> > (luckily we only have two deprecated commands and limited tests,
> >  otherwise extra commands will be M*N, say "number of deprecated
> >  command" * "number of test mirations")
> 
> Each test takes time, so adding tests make everything much slower.
> Notice that setting a new setting is fast.
> 
> This was the way that I understood Dave he wanted.

Do you mean every test is slow, or just migration tests?  Here I mean
to only test setting the parameters without doing real migration tests
(then query to make sure the settings were taking effect).  I assume
that should be fast too?  Thanks,

-- 
Peter Xu

Peter Xu <peterx@redhat.com> wrote:
> On Tue, Dec 26, 2017 at 08:51:10PM +0100, Juan Quintela wrote:
>> Peter Xu <peterx@redhat.com> wrote:
>> > On Fri, Dec 01, 2017 at 01:58:09PM +0100, Juan Quintela wrote:
>> >> We now test the deprecated commands everytime that we test the new
>> >> commands.  This makes unnecesary to add tests for deprecated commands.
>> >> 
>> >> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> >> ---
>> >>  tests/migration-test.c | 32 ++++++++++++++++++++++++++++----
>> >>  1 file changed, 28 insertions(+), 4 deletions(-)
>> >> 
>> >> diff --git a/tests/migration-test.c b/tests/migration-test.c
>> >> index 799e24ebc6..51f49c74e9 100644
>> >> --- a/tests/migration-test.c
>> >> +++ b/tests/migration-test.c
>> >> @@ -369,7 +369,7 @@ static void migrate_check_parameter(QTestState *who, const char *parameter,
>> >>      QDECREF(rsp);
>> >>  }
>> >>  
>> >> -static void migrate_set_downtime(QTestState *who, const double value)
>> >> +static void deprecated_set_downtime(QTestState *who, const double value)
>> >>  {
>> >>      QDict *rsp;
>> >>      gchar *cmd;
>> >> @@ -388,7 +388,7 @@ static void migrate_set_downtime(QTestState *who, const double value)
>> >>      g_free(expected);
>> >>  }
>> >>  
>> >> -static void migrate_set_speed(QTestState *who, const char *value)
>> >> +static void deprecated_set_speed(QTestState *who, const char *value)
>> >>  {
>> >>      QDict *rsp;
>> >>      gchar *cmd;
>> >> @@ -402,6 +402,30 @@ static void migrate_set_speed(QTestState *who, const char *value)
>> >>      migrate_check_parameter(who, "max-bandwidth", value);
>> >>  }
>> >>  
>> >> +static void migrate_set_parameter(QTestState *who, const char *parameter,
>> >> +                                  const char *value)
>> >> +{
>> >> +    QDict *rsp;
>> >> +    gchar *cmd;
>> >> +
>> >> +    if (strcmp(parameter, "downtime-limit") == 0) {
>> >> +        deprecated_set_downtime(who, 0.12345);
>> >> +    }
>> >> +
>> >> +    if (strcmp(parameter, "max-bandwidth") == 0) {
>> >> +        deprecated_set_speed(who, "12345");
>> >> +    }
>> >
>> > I'm fine with current approach, but I would really prefer to put them
>> > all into a standalone test, by just calling them one by one with some
>> > specific numbers and that's all.
>> 
>> That means another test (at least), and we have, also at least three
>> deprecated comands:
>> - migrate_set_speed
>> - migrate_set_downtime
>> - migrate_set_cache_size
>> 
>> And each test makes things slower.  So I *thought* it would we wiser to
>> just check _always_ use the deprecated an the not deprecated one.
>> 
>> > (luckily we only have two deprecated commands and limited tests,
>> >  otherwise extra commands will be M*N, say "number of deprecated
>> >  command" * "number of test mirations")
>> 
>> Each test takes time, so adding tests make everything much slower.
>> Notice that setting a new setting is fast.
>> 
>> This was the way that I understood Dave he wanted.
>
> Do you mean every test is slow, or just migration tests?

Each migration test adds around 2 seconds on my machine.  So I decided
that it was easier that each time that we check one command, we test the
deprecated and non-deprecated versions of the command.  Amount of time
added to the test is negigible, and we are sure that we always test both
functions.

If we ever remove the deprecated method, we can always remove only that
part of th etest.


> Here I mean
> to only test setting the parameters without doing real migration tests
> (then query to make sure the settings were taking effect).  I assume
> that should be fast too?  Thanks,

We could create a new test for that, but we need to start in on
source/destination, I thought it just made things more complicated.

If your preffer that way, please suggest how/what?

Thanks, Juan.

On Wed, Dec 27, 2017 at 10:41:02AM +0100, Juan Quintela wrote:
> Peter Xu <peterx@redhat.com> wrote:
> > On Tue, Dec 26, 2017 at 08:51:10PM +0100, Juan Quintela wrote:
> >> Peter Xu <peterx@redhat.com> wrote:
> >> > On Fri, Dec 01, 2017 at 01:58:09PM +0100, Juan Quintela wrote:
> >> >> We now test the deprecated commands everytime that we test the new
> >> >> commands.  This makes unnecesary to add tests for deprecated commands.
> >> >> 
> >> >> Signed-off-by: Juan Quintela <quintela@redhat.com>
> >> >> ---
> >> >>  tests/migration-test.c | 32 ++++++++++++++++++++++++++++----
> >> >>  1 file changed, 28 insertions(+), 4 deletions(-)
> >> >> 
> >> >> diff --git a/tests/migration-test.c b/tests/migration-test.c
> >> >> index 799e24ebc6..51f49c74e9 100644
> >> >> --- a/tests/migration-test.c
> >> >> +++ b/tests/migration-test.c
> >> >> @@ -369,7 +369,7 @@ static void migrate_check_parameter(QTestState *who, const char *parameter,
> >> >>      QDECREF(rsp);
> >> >>  }
> >> >>  
> >> >> -static void migrate_set_downtime(QTestState *who, const double value)
> >> >> +static void deprecated_set_downtime(QTestState *who, const double value)
> >> >>  {
> >> >>      QDict *rsp;
> >> >>      gchar *cmd;
> >> >> @@ -388,7 +388,7 @@ static void migrate_set_downtime(QTestState *who, const double value)
> >> >>      g_free(expected);
> >> >>  }
> >> >>  
> >> >> -static void migrate_set_speed(QTestState *who, const char *value)
> >> >> +static void deprecated_set_speed(QTestState *who, const char *value)
> >> >>  {
> >> >>      QDict *rsp;
> >> >>      gchar *cmd;
> >> >> @@ -402,6 +402,30 @@ static void migrate_set_speed(QTestState *who, const char *value)
> >> >>      migrate_check_parameter(who, "max-bandwidth", value);
> >> >>  }
> >> >>  
> >> >> +static void migrate_set_parameter(QTestState *who, const char *parameter,
> >> >> +                                  const char *value)
> >> >> +{
> >> >> +    QDict *rsp;
> >> >> +    gchar *cmd;
> >> >> +
> >> >> +    if (strcmp(parameter, "downtime-limit") == 0) {
> >> >> +        deprecated_set_downtime(who, 0.12345);
> >> >> +    }
> >> >> +
> >> >> +    if (strcmp(parameter, "max-bandwidth") == 0) {
> >> >> +        deprecated_set_speed(who, "12345");
> >> >> +    }
> >> >
> >> > I'm fine with current approach, but I would really prefer to put them
> >> > all into a standalone test, by just calling them one by one with some
> >> > specific numbers and that's all.
> >> 
> >> That means another test (at least), and we have, also at least three
> >> deprecated comands:
> >> - migrate_set_speed
> >> - migrate_set_downtime
> >> - migrate_set_cache_size
> >> 
> >> And each test makes things slower.  So I *thought* it would we wiser to
> >> just check _always_ use the deprecated an the not deprecated one.
> >> 
> >> > (luckily we only have two deprecated commands and limited tests,
> >> >  otherwise extra commands will be M*N, say "number of deprecated
> >> >  command" * "number of test mirations")
> >> 
> >> Each test takes time, so adding tests make everything much slower.
> >> Notice that setting a new setting is fast.
> >> 
> >> This was the way that I understood Dave he wanted.
> >
> > Do you mean every test is slow, or just migration tests?
> 
> Each migration test adds around 2 seconds on my machine.  So I decided
> that it was easier that each time that we check one command, we test the
> deprecated and non-deprecated versions of the command.  Amount of time
> added to the test is negigible, and we are sure that we always test both
> functions.
> 
> If we ever remove the deprecated method, we can always remove only that
> part of th etest.
> 
> 
> > Here I mean
> > to only test setting the parameters without doing real migration tests
> > (then query to make sure the settings were taking effect).  I assume
> > that should be fast too?  Thanks,
> 
> We could create a new test for that, but we need to start in on
> source/destination, I thought it just made things more complicated.
> 
> If your preffer that way, please suggest how/what?

I mean, an isolated test that only runs a single VM instance rather
than two (so no test_migrate_start() at all), but only: qtest_init(),
then qmp() to send the deprecated commands, then query to see whether
the values are set.  That's all.  Can that work for us?

-- 
Peter Xu

* Juan Quintela (quintela@redhat.com) wrote:
> We now test the deprecated commands everytime that we test the new
> commands.  This makes unnecesary to add tests for deprecated commands.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  tests/migration-test.c | 32 ++++++++++++++++++++++++++++----
>  1 file changed, 28 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index 799e24ebc6..51f49c74e9 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -369,7 +369,7 @@ static void migrate_check_parameter(QTestState *who, const char *parameter,
>      QDECREF(rsp);
>  }
>  
> -static void migrate_set_downtime(QTestState *who, const double value)
> +static void deprecated_set_downtime(QTestState *who, const double value)
>  {
>      QDict *rsp;
>      gchar *cmd;
> @@ -388,7 +388,7 @@ static void migrate_set_downtime(QTestState *who, const double value)
>      g_free(expected);
>  }
>  
> -static void migrate_set_speed(QTestState *who, const char *value)
> +static void deprecated_set_speed(QTestState *who, const char *value)
>  {
>      QDict *rsp;
>      gchar *cmd;
> @@ -402,6 +402,30 @@ static void migrate_set_speed(QTestState *who, const char *value)
>      migrate_check_parameter(who, "max-bandwidth", value);
>  }
>  
> +static void migrate_set_parameter(QTestState *who, const char *parameter,
> +                                  const char *value)
> +{
> +    QDict *rsp;
> +    gchar *cmd;
> +
> +    if (strcmp(parameter, "downtime-limit") == 0) {
> +        deprecated_set_downtime(who, 0.12345);
> +    }
> +
> +    if (strcmp(parameter, "max-bandwidth") == 0) {
> +        deprecated_set_speed(who, "12345");
> +    }

I find that odd; you call migrate_set_parameter to set a particular
value, but we set them to a different arbitrary value first?

Dave

> +    cmd = g_strdup_printf("{ 'execute': 'migrate-set-parameters',"
> +                          "'arguments': { '%s': %s } }",
> +                          parameter, value);
> +    rsp = qtest_qmp(who, cmd);
> +    g_free(cmd);
> +    g_assert(qdict_haskey(rsp, "return"));
> +    QDECREF(rsp);
> +    migrate_check_parameter(who, parameter, value);
> +}
> +
>  static void migrate_set_capability(QTestState *who, const char *capability,
>                                     const char *value)
>  {
> @@ -530,8 +554,8 @@ static void test_migrate(void)
>       * quickly, but that it doesn't complete precopy even on a slow
>       * machine, so also set the downtime.
>       */
> -    migrate_set_speed(from, "100000000");
> -    migrate_set_downtime(from, 0.001);
> +    migrate_set_parameter(from, "max-bandwidth", "100000000");
> +    migrate_set_parameter(from, "downtime-limit", "1");
>  
>      /* Wait for the first serial output from the source */
>      wait_for_serial("src_serial");
> -- 
> 2.14.3
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> We now test the deprecated commands everytime that we test the new
>> commands.  This makes unnecesary to add tests for deprecated commands.
>> 
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  tests/migration-test.c | 32 ++++++++++++++++++++++++++++----
>>  1 file changed, 28 insertions(+), 4 deletions(-)
>> 
>> diff --git a/tests/migration-test.c b/tests/migration-test.c
>> index 799e24ebc6..51f49c74e9 100644
>> --- a/tests/migration-test.c
>> +++ b/tests/migration-test.c
>> @@ -369,7 +369,7 @@ static void migrate_check_parameter(QTestState *who, const char *parameter,
>>      QDECREF(rsp);
>>  }
>>  
>> -static void migrate_set_downtime(QTestState *who, const double value)
>> +static void deprecated_set_downtime(QTestState *who, const double value)
>>  {
>>      QDict *rsp;
>>      gchar *cmd;
>> @@ -388,7 +388,7 @@ static void migrate_set_downtime(QTestState *who, const double value)
>>      g_free(expected);
>>  }
>>  
>> -static void migrate_set_speed(QTestState *who, const char *value)
>> +static void deprecated_set_speed(QTestState *who, const char *value)
>>  {
>>      QDict *rsp;
>>      gchar *cmd;
>> @@ -402,6 +402,30 @@ static void migrate_set_speed(QTestState *who, const char *value)
>>      migrate_check_parameter(who, "max-bandwidth", value);
>>  }
>>  
>> +static void migrate_set_parameter(QTestState *who, const char *parameter,
>> +                                  const char *value)
>> +{
>> +    QDict *rsp;
>> +    gchar *cmd;
>> +
>> +    if (strcmp(parameter, "downtime-limit") == 0) {
>> +        deprecated_set_downtime(who, 0.12345);
>> +    }
>> +
>> +    if (strcmp(parameter, "max-bandwidth") == 0) {
>> +        deprecated_set_speed(who, "12345");
>> +    }
>
> I find that odd; you call migrate_set_parameter to set a particular
> value, but we set them to a different arbitrary value first?

If I do:

migrate_deprecated_command(real_value)
migrate_non_deprecated_command(real_value)
  here the value is already set, so I am not sure that it is working.

This other way, if there is a _deprecated_ command,  I set it to a
known value, check that it was set correctly, and then set the real
value.

Later, Juan.

>
> Dave
>
>> +    cmd = g_strdup_printf("{ 'execute': 'migrate-set-parameters',"
>> +                          "'arguments': { '%s': %s } }",
>> +                          parameter, value);
>> +    rsp = qtest_qmp(who, cmd);
>> +    g_free(cmd);
>> +    g_assert(qdict_haskey(rsp, "return"));
>> +    QDECREF(rsp);
>> +    migrate_check_parameter(who, parameter, value);
>> +}
>> +
>>  static void migrate_set_capability(QTestState *who, const char *capability,
>>                                     const char *value)
>>  {
>> @@ -530,8 +554,8 @@ static void test_migrate(void)
>>       * quickly, but that it doesn't complete precopy even on a slow
>>       * machine, so also set the downtime.
>>       */
>> -    migrate_set_speed(from, "100000000");
>> -    migrate_set_downtime(from, 0.001);
>> +    migrate_set_parameter(from, "max-bandwidth", "100000000");
>> +    migrate_set_parameter(from, "downtime-limit", "1");
>>  
>>      /* Wait for the first serial output from the source */
>>      wait_for_serial("src_serial");
>> -- 
>> 2.14.3
>> 
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK