Re: [PATCH v2] tests/acpi/bios-tables-test: do not write new blobs unless there are changes

[Ani Sinha]

> On 21-Nov-2023, at 12:39 PM, Ani Sinha <anisinha@redhat.com> wrote:
> 
> 
> 
>> On 16-Nov-2023, at 11:30 AM, Ani Sinha <anisinha@redhat.com> wrote:
>> 
>> 
>> 
>>> On 07-Nov-2023, at 10:19 AM, Ani Sinha <anisinha@redhat.com> wrote:
>>> 
>>> When dumping table blobs using rebuild-expected-aml.sh, table blobs from all
>>> test variants are dumped regardless of whether there are any actual changes 
>>> to
>>> the tables or not. This creates lot of new files for various test variants 
>>> that
>>> are not part of the git repository. This is because we do not check in all 
>>> table
>>> blobs for all test variants into the repository. Only those blobs for those
>>> variants that are different from the generic test-variant agnostic blob are
>>> checked in.
>>> 
>>> This change makes the test smarter by checking if at all there are any 
>>> changes
>>> in the tables from the checked-in gold master blobs and take actions
>>> accordingly.
>>> 
>>> When there are no changes:
>>> - No new table blobs would be written.
>>> - Existing table blobs will be refreshed (git diff will show no changes).
>>> When there are changes:
>>> - New table blob files will be dumped.
>>> - Existing table blobs will be refreshed (git diff will show that the files
>>> changed, asl diff will show the actual changes).
>>> When new tables are introduced:
>>> - Zero byte empty file blobs for new tables as instructed in the header of
>>> bios-tables-test.c will be regenerated to actual table blobs.
>>> 
>>> This would make analyzing changes to tables less confusing and there would
>>> be no need to clean useless untracked files when there are no table changes.
>>> 
>>> CC: peter.maydell@linaro.org
>>> Signed-off-by: Ani Sinha <anisinha@redhat.com>
>>> ---
>>> tests/qtest/bios-tables-test.c | 14 +++++++++++++-
>>> 1 file changed, 13 insertions(+), 1 deletion(-)
>>> 
>>> changelog:
>>> v2: commit description updated to make things a little clearer.
>>>  No actual changes.
>> 
>> Ping ...
> 
> Ping again ...

Ping …

> 
>> 
>>> 
>>> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
>>> index 9f4bc15aab..743b509e93 100644
>>> --- a/tests/qtest/bios-tables-test.c
>>> +++ b/tests/qtest/bios-tables-test.c
>>> @@ -109,6 +109,7 @@ static const char *iasl;
>>> #endif
>>> 
>>> static int verbosity_level;
>>> +static GArray *load_expected_aml(test_data *data);
>>> 
>>> static bool compare_signature(const AcpiSdtTable *sdt, const char 
>>> *signature)
>>> {
>>> @@ -241,21 +242,32 @@ static void test_acpi_fadt_table(test_data *data)
>>> 
>>> static void dump_aml_files(test_data *data, bool rebuild)
>>> {
>>> -    AcpiSdtTable *sdt;
>>> +    AcpiSdtTable *sdt, *exp_sdt;
>>>   GError *error = NULL;
>>>   gchar *aml_file = NULL;
>>> +    test_data exp_data = {};
>>>   gint fd;
>>>   ssize_t ret;
>>>   int i;
>>> 
>>> +    exp_data.tables = load_expected_aml(data);
>>>   for (i = 0; i < data->tables->len; ++i) {
>>>       const char *ext = data->variant ? data->variant : "";
>>>       sdt = &g_array_index(data->tables, AcpiSdtTable, i);
>>> +        exp_sdt = &g_array_index(exp_data.tables, AcpiSdtTable, i);
>>>       g_assert(sdt->aml);
>>> +        g_assert(exp_sdt->aml);
>>> 
>>>       if (rebuild) {
>>>           aml_file = g_strdup_printf("%s/%s/%.4s%s", data_dir, 
>>> data->machine,
>>>                                      sdt->aml, ext);
>>> +            if (!g_file_test(aml_file, G_FILE_TEST_EXISTS) &&
>>> +                sdt->aml_len == exp_sdt->aml_len &&
>>> +                !memcmp(sdt->aml, exp_sdt->aml, sdt->aml_len)) {
>>> +                /* identical tables, no need to write new files */
>>> +                g_free(aml_file);
>>> +                continue;
>>> +            }
>>>           fd = g_open(aml_file, O_WRONLY|O_TRUNC|O_CREAT,
>>>                       S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH);
>>>           if (fd < 0) {
>>> -- 
>>> 2.42.0