diff mbox series

[v7,09/10] ACPI ERST: bios-tables-test.c steps 1 and 2

Message ID 1633626876-12115-10-git-send-email-eric.devolder@oracle.com (mailing list archive)
State New, archived
Headers show
Series acpi: Error Record Serialization Table, ERST, support for QEMU | expand

Commit Message

Eric DeVolder Oct. 7, 2021, 5:14 p.m. UTC
Following the guidelines in tests/qtest/bios-tables-test.c, this
change adds empty placeholder files per step 1 for the new ERST
table, and excludes resulting changed files in bios-tables-test-allowed-diff.h
per step 2.

Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
Acked-by: Igor Mammedov <imammedo@redhat.com>
---
 tests/data/acpi/microvm/ERST.pcie           | 0
 tests/data/acpi/pc/DSDT.acpierst            | 0
 tests/data/acpi/pc/ERST                     | 0
 tests/data/acpi/q35/DSDT.acpierst           | 0
 tests/data/acpi/q35/ERST                    | 0
 tests/qtest/bios-tables-test-allowed-diff.h | 5 +++++
 6 files changed, 5 insertions(+)
 create mode 100644 tests/data/acpi/microvm/ERST.pcie
 create mode 100644 tests/data/acpi/pc/DSDT.acpierst
 create mode 100644 tests/data/acpi/pc/ERST
 create mode 100644 tests/data/acpi/q35/DSDT.acpierst
 create mode 100644 tests/data/acpi/q35/ERST

Comments

Ani Sinha Oct. 8, 2021, 1:17 a.m. UTC | #1
The ordering of the patches is wrong!

(a) First, you need this patch so that the test framework will ignore
changes
to the table blobs that you specify here to explicitly ignore.

(b) Then you need the patch that actually contains the test you wrote
(patch
8). Now because you have previously ignored the changes to ACPI tables
that the test would modify, the tests would continue to ignore them and
hence it would not fail "make check".

(c) Lastly, you need patch 10 where you empty out the list of table blobs
to
ignore and update the blobs.

Please also make sure you only update those table blobs that you
explicitly ignored in step (a).


On Thu, 7 Oct 2021, Eric DeVolder wrote:

> Following the guidelines in tests/qtest/bios-tables-test.c, this
> change adds empty placeholder files per step 1 for the new ERST
> table, and excludes resulting changed files in bios-tables-test-allowed-diff.h
> per step 2.
>
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> Acked-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  tests/data/acpi/microvm/ERST.pcie           | 0
>  tests/data/acpi/pc/DSDT.acpierst            | 0
>  tests/data/acpi/pc/ERST                     | 0
>  tests/data/acpi/q35/DSDT.acpierst           | 0
>  tests/data/acpi/q35/ERST                    | 0
>  tests/qtest/bios-tables-test-allowed-diff.h | 5 +++++
>  6 files changed, 5 insertions(+)
>  create mode 100644 tests/data/acpi/microvm/ERST.pcie
>  create mode 100644 tests/data/acpi/pc/DSDT.acpierst
>  create mode 100644 tests/data/acpi/pc/ERST
>  create mode 100644 tests/data/acpi/q35/DSDT.acpierst
>  create mode 100644 tests/data/acpi/q35/ERST
>
> diff --git a/tests/data/acpi/microvm/ERST.pcie b/tests/data/acpi/microvm/ERST.pcie
> new file mode 100644
> index 0000000..e69de29
> diff --git a/tests/data/acpi/pc/DSDT.acpierst b/tests/data/acpi/pc/DSDT.acpierst
> new file mode 100644
> index 0000000..e69de29
> diff --git a/tests/data/acpi/pc/ERST b/tests/data/acpi/pc/ERST
> new file mode 100644
> index 0000000..e69de29
> diff --git a/tests/data/acpi/q35/DSDT.acpierst b/tests/data/acpi/q35/DSDT.acpierst
> new file mode 100644
> index 0000000..e69de29
> diff --git a/tests/data/acpi/q35/ERST b/tests/data/acpi/q35/ERST
> new file mode 100644
> index 0000000..e69de29
> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
> index dfb8523..c06241a 100644
> --- a/tests/qtest/bios-tables-test-allowed-diff.h
> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> @@ -1 +1,6 @@
>  /* List of comma-separated changed AML files to ignore */
> +"tests/data/acpi/pc/DSDT.acpierst",
> +"tests/data/acpi/pc/ERST",
> +"tests/data/acpi/q35/DSDT.acpierst",
> +"tests/data/acpi/q35/ERST",
> +"tests/data/acpi/microvm/ERST.pcie",
> --
> 1.8.3.1
>
>
Eric DeVolder Oct. 14, 2021, 7:34 p.m. UTC | #2
On 10/7/21 20:17, Ani Sinha wrote:
> The ordering of the patches is wrong!
> 
> (a) First, you need this patch so that the test framework will ignore
> changes
> to the table blobs that you specify here to explicitly ignore.
> 
> (b) Then you need the patch that actually contains the test you wrote
> (patch
> 8). Now because you have previously ignored the changes to ACPI tables
> that the test would modify, the tests would continue to ignore them and
> hence it would not fail "make check".
> 
> (c) Lastly, you need patch 10 where you empty out the list of table blobs
> to
> ignore and update the blobs.

Thanks for explaining; it sounds like the steps outlined in bios-tables-test.c.
I moved this patch based on previous review, perhaps I misunderstand what was being requested.
At any rate, I've moved this to be the first patch, per bios-tables-test.c

> 
> Please also make sure you only update those table blobs that you
> explicitly ignored in step (a).

Done!


> 
> 
> On Thu, 7 Oct 2021, Eric DeVolder wrote:
> 
>> Following the guidelines in tests/qtest/bios-tables-test.c, this
>> change adds empty placeholder files per step 1 for the new ERST
>> table, and excludes resulting changed files in bios-tables-test-allowed-diff.h
>> per step 2.
>>
>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
>> Acked-by: Igor Mammedov <imammedo@redhat.com>
>> ---
>>   tests/data/acpi/microvm/ERST.pcie           | 0
>>   tests/data/acpi/pc/DSDT.acpierst            | 0
>>   tests/data/acpi/pc/ERST                     | 0
>>   tests/data/acpi/q35/DSDT.acpierst           | 0
>>   tests/data/acpi/q35/ERST                    | 0
>>   tests/qtest/bios-tables-test-allowed-diff.h | 5 +++++
>>   6 files changed, 5 insertions(+)
>>   create mode 100644 tests/data/acpi/microvm/ERST.pcie
>>   create mode 100644 tests/data/acpi/pc/DSDT.acpierst
>>   create mode 100644 tests/data/acpi/pc/ERST
>>   create mode 100644 tests/data/acpi/q35/DSDT.acpierst
>>   create mode 100644 tests/data/acpi/q35/ERST
>>
>> diff --git a/tests/data/acpi/microvm/ERST.pcie b/tests/data/acpi/microvm/ERST.pcie
>> new file mode 100644
>> index 0000000..e69de29
>> diff --git a/tests/data/acpi/pc/DSDT.acpierst b/tests/data/acpi/pc/DSDT.acpierst
>> new file mode 100644
>> index 0000000..e69de29
>> diff --git a/tests/data/acpi/pc/ERST b/tests/data/acpi/pc/ERST
>> new file mode 100644
>> index 0000000..e69de29
>> diff --git a/tests/data/acpi/q35/DSDT.acpierst b/tests/data/acpi/q35/DSDT.acpierst
>> new file mode 100644
>> index 0000000..e69de29
>> diff --git a/tests/data/acpi/q35/ERST b/tests/data/acpi/q35/ERST
>> new file mode 100644
>> index 0000000..e69de29
>> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
>> index dfb8523..c06241a 100644
>> --- a/tests/qtest/bios-tables-test-allowed-diff.h
>> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
>> @@ -1 +1,6 @@
>>   /* List of comma-separated changed AML files to ignore */
>> +"tests/data/acpi/pc/DSDT.acpierst",
>> +"tests/data/acpi/pc/ERST",
>> +"tests/data/acpi/q35/DSDT.acpierst",
>> +"tests/data/acpi/q35/ERST",
>> +"tests/data/acpi/microvm/ERST.pcie",
>> --
>> 1.8.3.1
>>
>>
diff mbox series

Patch

diff --git a/tests/data/acpi/microvm/ERST.pcie b/tests/data/acpi/microvm/ERST.pcie
new file mode 100644
index 0000000..e69de29
diff --git a/tests/data/acpi/pc/DSDT.acpierst b/tests/data/acpi/pc/DSDT.acpierst
new file mode 100644
index 0000000..e69de29
diff --git a/tests/data/acpi/pc/ERST b/tests/data/acpi/pc/ERST
new file mode 100644
index 0000000..e69de29
diff --git a/tests/data/acpi/q35/DSDT.acpierst b/tests/data/acpi/q35/DSDT.acpierst
new file mode 100644
index 0000000..e69de29
diff --git a/tests/data/acpi/q35/ERST b/tests/data/acpi/q35/ERST
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523..c06241a 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,6 @@ 
 /* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/pc/DSDT.acpierst",
+"tests/data/acpi/pc/ERST",
+"tests/data/acpi/q35/DSDT.acpierst",
+"tests/data/acpi/q35/ERST",
+"tests/data/acpi/microvm/ERST.pcie",