diff mbox series

[v1,5/7] tests/migration-test: don't spam the logs when we fail

Message ID 20190712111849.9006-6-alex.bennee@linaro.org
State New
Headers show
Series testing/next (docker, win-cross) | expand

Commit Message

Alex Bennée July 12, 2019, 11:18 a.m. UTC
Quite often the information about which test failed is hidden by the
wall of repeated failures for each page. Stop outputting the error
after 10 bad pages and just summarise the total damage at the end.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/migration-test.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

Comments

Laurent Vivier July 12, 2019, 11:22 a.m. UTC | #1
On 12/07/2019 13:18, Alex Bennée wrote:
> Quite often the information about which test failed is hidden by the
> wall of repeated failures for each page. Stop outputting the error
> after 10 bad pages and just summarise the total damage at the end.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  tests/migration-test.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index b6434628e1c..ce041f80c2a 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -308,7 +308,7 @@ static void check_guests_ram(QTestState *who)
>      uint8_t first_byte;
>      uint8_t last_byte;
>      bool hit_edge = false;
> -    bool bad = false;
> +    int bad = 0;
>  
>      qtest_memread(who, start_address, &first_byte, 1);
>      last_byte = first_byte;
> @@ -327,15 +327,20 @@ static void check_guests_ram(QTestState *who)
>                  hit_edge = true;
>                  last_byte = b;
>              } else {
> -                fprintf(stderr, "Memory content inconsistency at %x"
> -                                " first_byte = %x last_byte = %x current = %x"
> -                                " hit_edge = %x\n",
> -                                address, first_byte, last_byte, b, hit_edge);
> -                bad = true;
> +                bad++;
> +                if (bad <= 10) {
> +                    fprintf(stderr, "Memory content inconsistency at %x"
> +                            " first_byte = %x last_byte = %x current = %x"
> +                            " hit_edge = %x\n",
> +                            address, first_byte, last_byte, b, hit_edge);
> +                }
>              }
>          }
>      }
> -    g_assert_false(bad);
> +    if (bad >= 10) {
> +        fprintf(stderr, "and in another %d pages", bad);

"bad - 10" as you have already displayed 10 errors.

Thanks,
Laurent
Thomas Huth July 12, 2019, 11:23 a.m. UTC | #2
On 12/07/2019 13.18, Alex Bennée wrote:
> Quite often the information about which test failed is hidden by the
> wall of repeated failures for each page. Stop outputting the error
> after 10 bad pages and just summarise the total damage at the end.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  tests/migration-test.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index b6434628e1c..ce041f80c2a 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -308,7 +308,7 @@ static void check_guests_ram(QTestState *who)
>      uint8_t first_byte;
>      uint8_t last_byte;
>      bool hit_edge = false;
> -    bool bad = false;
> +    int bad = 0;
>  
>      qtest_memread(who, start_address, &first_byte, 1);
>      last_byte = first_byte;
> @@ -327,15 +327,20 @@ static void check_guests_ram(QTestState *who)
>                  hit_edge = true;
>                  last_byte = b;
>              } else {
> -                fprintf(stderr, "Memory content inconsistency at %x"
> -                                " first_byte = %x last_byte = %x current = %x"
> -                                " hit_edge = %x\n",
> -                                address, first_byte, last_byte, b, hit_edge);
> -                bad = true;
> +                bad++;
> +                if (bad <= 10) {
> +                    fprintf(stderr, "Memory content inconsistency at %x"
> +                            " first_byte = %x last_byte = %x current = %x"
> +                            " hit_edge = %x\n",
> +                            address, first_byte, last_byte, b, hit_edge);
> +                }
>              }
>          }
>      }
> -    g_assert_false(bad);
> +    if (bad >= 10) {
> +        fprintf(stderr, "and in another %d pages", bad);
> +    }
> +    g_assert(bad == 0);
>  }
>  
>  static void cleanup(const char *filename)

Good idea.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Dr. David Alan Gilbert July 12, 2019, 11:42 a.m. UTC | #3
* Alex Bennée (alex.bennee@linaro.org) wrote:
> Quite often the information about which test failed is hidden by the
> wall of repeated failures for each page. Stop outputting the error
> after 10 bad pages and just summarise the total damage at the end.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

Yep, occasionally you do find you do want the full set to see what's
going on, but this is true most of the time.

With 10 you should be able to see if it's a single page that's wrong or
the flip gone wrong.

> ---
>  tests/migration-test.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index b6434628e1c..ce041f80c2a 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -308,7 +308,7 @@ static void check_guests_ram(QTestState *who)
>      uint8_t first_byte;
>      uint8_t last_byte;
>      bool hit_edge = false;
> -    bool bad = false;
> +    int bad = 0;
>  
>      qtest_memread(who, start_address, &first_byte, 1);
>      last_byte = first_byte;
> @@ -327,15 +327,20 @@ static void check_guests_ram(QTestState *who)
>                  hit_edge = true;
>                  last_byte = b;
>              } else {
> -                fprintf(stderr, "Memory content inconsistency at %x"
> -                                " first_byte = %x last_byte = %x current = %x"
> -                                " hit_edge = %x\n",
> -                                address, first_byte, last_byte, b, hit_edge);
> -                bad = true;
> +                bad++;
> +                if (bad <= 10) {
> +                    fprintf(stderr, "Memory content inconsistency at %x"
> +                            " first_byte = %x last_byte = %x current = %x"
> +                            " hit_edge = %x\n",
> +                            address, first_byte, last_byte, b, hit_edge);
> +                }
>              }
>          }
>      }
> -    g_assert_false(bad);
> +    if (bad >= 10) {
> +        fprintf(stderr, "and in another %d pages", bad);
> +    }

as Laurent says, the 'another' would need -10 but other than that:


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

> +    g_assert(bad == 0);
>  }
>  
>  static void cleanup(const char *filename)
> -- 
> 2.20.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Alex Bennée July 12, 2019, 1:35 p.m. UTC | #4
Laurent Vivier <lvivier@redhat.com> writes:

> On 12/07/2019 13:18, Alex Bennée wrote:
>> Quite often the information about which test failed is hidden by the
>> wall of repeated failures for each page. Stop outputting the error
>> after 10 bad pages and just summarise the total damage at the end.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  tests/migration-test.c | 19 ++++++++++++-------
>>  1 file changed, 12 insertions(+), 7 deletions(-)
>>
>> diff --git a/tests/migration-test.c b/tests/migration-test.c
>> index b6434628e1c..ce041f80c2a 100644
>> --- a/tests/migration-test.c
>> +++ b/tests/migration-test.c
>> @@ -308,7 +308,7 @@ static void check_guests_ram(QTestState *who)
>>      uint8_t first_byte;
>>      uint8_t last_byte;
>>      bool hit_edge = false;
>> -    bool bad = false;
>> +    int bad = 0;
>>
>>      qtest_memread(who, start_address, &first_byte, 1);
>>      last_byte = first_byte;
>> @@ -327,15 +327,20 @@ static void check_guests_ram(QTestState *who)
>>                  hit_edge = true;
>>                  last_byte = b;
>>              } else {
>> -                fprintf(stderr, "Memory content inconsistency at %x"
>> -                                " first_byte = %x last_byte = %x current = %x"
>> -                                " hit_edge = %x\n",
>> -                                address, first_byte, last_byte, b, hit_edge);
>> -                bad = true;
>> +                bad++;
>> +                if (bad <= 10) {
>> +                    fprintf(stderr, "Memory content inconsistency at %x"
>> +                            " first_byte = %x last_byte = %x current = %x"
>> +                            " hit_edge = %x\n",
>> +                            address, first_byte, last_byte, b, hit_edge);
>> +                }
>>              }
>>          }
>>      }
>> -    g_assert_false(bad);
>> +    if (bad >= 10) {
>> +        fprintf(stderr, "and in another %d pages", bad);
>
> "bad - 10" as you have already displayed 10 errors.

Will do.

>
> Thanks,
> Laurent


--
Alex Bennée
Laurent Vivier July 12, 2019, 1:49 p.m. UTC | #5
On 12/07/2019 15:35, Alex Bennée wrote:
> 
> Laurent Vivier <lvivier@redhat.com> writes:
> 
>> On 12/07/2019 13:18, Alex Bennée wrote:
>>> Quite often the information about which test failed is hidden by the
>>> wall of repeated failures for each page. Stop outputting the error
>>> after 10 bad pages and just summarise the total damage at the end.
>>>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> ---
>>>  tests/migration-test.c | 19 ++++++++++++-------
>>>  1 file changed, 12 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/tests/migration-test.c b/tests/migration-test.c
>>> index b6434628e1c..ce041f80c2a 100644
>>> --- a/tests/migration-test.c
>>> +++ b/tests/migration-test.c
>>> @@ -308,7 +308,7 @@ static void check_guests_ram(QTestState *who)
>>>      uint8_t first_byte;
>>>      uint8_t last_byte;
>>>      bool hit_edge = false;
>>> -    bool bad = false;
>>> +    int bad = 0;
>>>
>>>      qtest_memread(who, start_address, &first_byte, 1);
>>>      last_byte = first_byte;
>>> @@ -327,15 +327,20 @@ static void check_guests_ram(QTestState *who)
>>>                  hit_edge = true;
>>>                  last_byte = b;
>>>              } else {
>>> -                fprintf(stderr, "Memory content inconsistency at %x"
>>> -                                " first_byte = %x last_byte = %x current = %x"
>>> -                                " hit_edge = %x\n",
>>> -                                address, first_byte, last_byte, b, hit_edge);
>>> -                bad = true;
>>> +                bad++;
>>> +                if (bad <= 10) {
>>> +                    fprintf(stderr, "Memory content inconsistency at %x"
>>> +                            " first_byte = %x last_byte = %x current = %x"
>>> +                            " hit_edge = %x\n",
>>> +                            address, first_byte, last_byte, b, hit_edge);
>>> +                }
>>>              }
>>>          }
>>>      }
>>> -    g_assert_false(bad);
>>> +    if (bad >= 10) {
>>> +        fprintf(stderr, "and in another %d pages", bad);
>>
>> "bad - 10" as you have already displayed 10 errors.
> 
> Will do.

You can add my:

Reviewed-by: Laurent Vivier <lvivier@redhat.com>

Thanks,
Laurent
diff mbox series

Patch

diff --git a/tests/migration-test.c b/tests/migration-test.c
index b6434628e1c..ce041f80c2a 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -308,7 +308,7 @@  static void check_guests_ram(QTestState *who)
     uint8_t first_byte;
     uint8_t last_byte;
     bool hit_edge = false;
-    bool bad = false;
+    int bad = 0;
 
     qtest_memread(who, start_address, &first_byte, 1);
     last_byte = first_byte;
@@ -327,15 +327,20 @@  static void check_guests_ram(QTestState *who)
                 hit_edge = true;
                 last_byte = b;
             } else {
-                fprintf(stderr, "Memory content inconsistency at %x"
-                                " first_byte = %x last_byte = %x current = %x"
-                                " hit_edge = %x\n",
-                                address, first_byte, last_byte, b, hit_edge);
-                bad = true;
+                bad++;
+                if (bad <= 10) {
+                    fprintf(stderr, "Memory content inconsistency at %x"
+                            " first_byte = %x last_byte = %x current = %x"
+                            " hit_edge = %x\n",
+                            address, first_byte, last_byte, b, hit_edge);
+                }
             }
         }
     }
-    g_assert_false(bad);
+    if (bad >= 10) {
+        fprintf(stderr, "and in another %d pages", bad);
+    }
+    g_assert(bad == 0);
 }
 
 static void cleanup(const char *filename)