[Qemu-devel] [PATCH 2/2] tests: acpi: do not skip tests when IASL is not installed

Igor Mammedov posted 2 patches 6 years, 7 months ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Thomas Huth <thuth@redhat.com>, Laurent Vivier <lvivier@redhat.com>, Igor Mammedov <imammedo@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>
There is a newer version of this series
[Qemu-devel] [PATCH 2/2] tests: acpi: do not skip tests when IASL is not installed
Posted by Igor Mammedov 6 years, 7 months ago
tests do binary comparition so we can check tables without
IASL. Move IASL condition right before decompilation step
and skip it if IASL is not installed.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 tests/bios-tables-test.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 13bd166b81..a356ac3489 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -389,6 +389,14 @@ static void test_acpi_asl(test_data *data)
         all_tables_match = all_tables_match &&
             test_acpi_find_diff_allowed(exp_sdt);
 
+        /*
+         * don't try to decompile if IASL isn't present, in this case user
+         * will just 'get binary file mismatch' warnings and test failure
+         */
+        if (!iasl) {
+            continue;
+        }
+
         err = load_asl(data->tables, sdt);
         asl = normalize_asl(sdt->asl);
 
@@ -431,6 +439,11 @@ static void test_acpi_asl(test_data *data)
         g_string_free(asl, true);
         g_string_free(exp_asl, true);
     }
+    if (!iasl && !all_tables_match) {
+        fprintf(stderr, "to see ASL diff between mismatched files install IASL,"
+                " rebuild QEMU from scratch and re-run tests with V=1"
+                " environment variable set");
+    }
     g_assert(all_tables_match);
 
     free_test_data(&exp_data);
@@ -599,7 +612,7 @@ static void test_acpi_one(const char *params, test_data *data)
 
     if (getenv(ACPI_REBUILD_EXPECTED_AML)) {
         dump_aml_files(data, true);
-    } else if (iasl) {
+    } else {
         test_acpi_asl(data);
     }
 
-- 
2.18.1


Re: [Qemu-devel] [PATCH 2/2] tests: acpi: do not skip tests when IASL is not installed
Posted by Auger Eric 6 years, 7 months ago
Hi,

On 7/4/19 10:27 AM, Igor Mammedov wrote:
> tests do binary comparition so we can check tables without
comparison
> IASL. Move IASL condition right before decompilation step
> and skip it if IASL is not installed.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  tests/bios-tables-test.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
> index 13bd166b81..a356ac3489 100644
> --- a/tests/bios-tables-test.c
> +++ b/tests/bios-tables-test.c
> @@ -389,6 +389,14 @@ static void test_acpi_asl(test_data *data)
>          all_tables_match = all_tables_match &&
>              test_acpi_find_diff_allowed(exp_sdt);
>  
> +        /*
> +         * don't try to decompile if IASL isn't present, in this case user
> +         * will just 'get binary file mismatch' warnings and test failure
> +         */
> +        if (!iasl) {
> +            continue;
> +        }
> +
>          err = load_asl(data->tables, sdt);
>          asl = normalize_asl(sdt->asl);
>  
> @@ -431,6 +439,11 @@ static void test_acpi_asl(test_data *data)
>          g_string_free(asl, true);
>          g_string_free(exp_asl, true);
>      }
> +    if (!iasl && !all_tables_match) {
> +        fprintf(stderr, "to see ASL diff between mismatched files install IASL,"
> +                " rebuild QEMU from scratch and re-run tests with V=1"
> +                " environment variable set");
> +    }
>      g_assert(all_tables_match);
>  
>      free_test_data(&exp_data);
> @@ -599,7 +612,7 @@ static void test_acpi_one(const char *params, test_data *data)
>  
>      if (getenv(ACPI_REBUILD_EXPECTED_AML)) {
>          dump_aml_files(data, true);
> -    } else if (iasl) {
> +    } else {
>          test_acpi_asl(data);
>      }
>  
> 
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric

Re: [Qemu-devel] [PATCH 2/2] tests: acpi: do not skip tests when IASL is not installed
Posted by Philippe Mathieu-Daudé 6 years, 7 months ago
On 7/4/19 10:27 AM, Igor Mammedov wrote:
> tests do binary comparition so we can check tables without
> IASL. Move IASL condition right before decompilation step
> and skip it if IASL is not installed.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  tests/bios-tables-test.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
> index 13bd166b81..a356ac3489 100644
> --- a/tests/bios-tables-test.c
> +++ b/tests/bios-tables-test.c
> @@ -389,6 +389,14 @@ static void test_acpi_asl(test_data *data)
>          all_tables_match = all_tables_match &&
>              test_acpi_find_diff_allowed(exp_sdt);
>  
> +        /*
> +         * don't try to decompile if IASL isn't present, in this case user
> +         * will just 'get binary file mismatch' warnings and test failure
> +         */
> +        if (!iasl) {
> +            continue;
> +        }
> +
>          err = load_asl(data->tables, sdt);
>          asl = normalize_asl(sdt->asl);
>  
> @@ -431,6 +439,11 @@ static void test_acpi_asl(test_data *data)
>          g_string_free(asl, true);
>          g_string_free(exp_asl, true);
>      }
> +    if (!iasl && !all_tables_match) {
> +        fprintf(stderr, "to see ASL diff between mismatched files install IASL,"
> +                " rebuild QEMU from scratch and re-run tests with V=1"
> +                " environment variable set");

I guess remember Thomas asked to use g_printerr() instead of fprintf()
in tests/*.c.

Anyway,
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> +    }
>      g_assert(all_tables_match);
>  
>      free_test_data(&exp_data);
> @@ -599,7 +612,7 @@ static void test_acpi_one(const char *params, test_data *data)
>  
>      if (getenv(ACPI_REBUILD_EXPECTED_AML)) {
>          dump_aml_files(data, true);
> -    } else if (iasl) {
> +    } else {
>          test_acpi_asl(data);
>      }
>  
> 

Re: [Qemu-devel] [PATCH 2/2] tests: acpi: do not skip tests when IASL is not installed
Posted by Igor Mammedov 6 years, 7 months ago
On Thu, 4 Jul 2019 11:33:19 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> On 7/4/19 10:27 AM, Igor Mammedov wrote:
> > tests do binary comparition so we can check tables without
> > IASL. Move IASL condition right before decompilation step
> > and skip it if IASL is not installed.
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  tests/bios-tables-test.c | 15 ++++++++++++++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
> > index 13bd166b81..a356ac3489 100644
> > --- a/tests/bios-tables-test.c
> > +++ b/tests/bios-tables-test.c
> > @@ -389,6 +389,14 @@ static void test_acpi_asl(test_data *data)
> >          all_tables_match = all_tables_match &&
> >              test_acpi_find_diff_allowed(exp_sdt);
> >  
> > +        /*
> > +         * don't try to decompile if IASL isn't present, in this case user
> > +         * will just 'get binary file mismatch' warnings and test failure
> > +         */
> > +        if (!iasl) {
> > +            continue;
> > +        }
> > +
> >          err = load_asl(data->tables, sdt);
> >          asl = normalize_asl(sdt->asl);
> >  
> > @@ -431,6 +439,11 @@ static void test_acpi_asl(test_data *data)
> >          g_string_free(asl, true);
> >          g_string_free(exp_asl, true);
> >      }
> > +    if (!iasl && !all_tables_match) {
> > +        fprintf(stderr, "to see ASL diff between mismatched files install IASL,"
> > +                " rebuild QEMU from scratch and re-run tests with V=1"
> > +                " environment variable set");  
> 
> I guess remember Thomas asked to use g_printerr() instead of fprintf()
whole file uses fprintf().
I'd rather send extra patch on top that consistently changes
test to g_printerr()

> in tests/*.c.
> 
> Anyway,
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Thanks!

> 
> > +    }
> >      g_assert(all_tables_match);
> >  
> >      free_test_data(&exp_data);
> > @@ -599,7 +612,7 @@ static void test_acpi_one(const char *params, test_data *data)
> >  
> >      if (getenv(ACPI_REBUILD_EXPECTED_AML)) {
> >          dump_aml_files(data, true);
> > -    } else if (iasl) {
> > +    } else {
> >          test_acpi_asl(data);
> >      }
> >  
> >   
> 


[Qemu-devel] [PATCH v2 2/2] tests: acpi: do not skip tests when IASL is not installed
Posted by Igor Mammedov 6 years, 7 months ago
tests do binary comparision so we can check tables without
IASL. Move IASL condition right before decompilation step
and skip it if IASL is not installed.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v2:
 - fix typo in commit message
     Eric Auger <eric.auger@redhat.com>

 tests/bios-tables-test.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 13bd166b81..a356ac3489 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -389,6 +389,14 @@ static void test_acpi_asl(test_data *data)
         all_tables_match = all_tables_match &&
             test_acpi_find_diff_allowed(exp_sdt);
 
+        /*
+         *  don't try to decompile if IASL isn't present, in this case user
+         * will just 'get binary file mismatch' warnings and test failure
+         */
+        if (!iasl) {
+            continue;
+        }
+
         err = load_asl(data->tables, sdt);
         asl = normalize_asl(sdt->asl);
 
@@ -431,6 +439,11 @@ static void test_acpi_asl(test_data *data)
         g_string_free(asl, true);
         g_string_free(exp_asl, true);
     }
+    if (!iasl && !all_tables_match) {
+        fprintf(stderr, "to see ASL diff between mismatched files install IASL,"
+                " rebuild QEMU from scratch and re-run tests with V=1"
+                " environment variable set");
+    }
     g_assert(all_tables_match);
 
     free_test_data(&exp_data);
@@ -599,7 +612,7 @@ static void test_acpi_one(const char *params, test_data *data)
 
     if (getenv(ACPI_REBUILD_EXPECTED_AML)) {
         dump_aml_files(data, true);
-    } else if (iasl) {
+    } else {
         test_acpi_asl(data);
     }
 
-- 
2.18.1