> On 06-Nov-2023, at 8:43 PM, Ani Sinha <anisi...@redhat.com> wrote:
>
>
>
>> On 06-Nov-2023, at 7:45 PM, Igor Mammedov <imamm...@redhat.com> wrote:
>>
>> On Thu, 2 Nov 2023 13:46:24 +0530
>> Ani Sinha <anisi...@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.
>>
>>> If there are no changes, no new files are written for test variants.
>>> However, existing files continue to be overwritten regardless of whether
>>> there are changes.
>>> Hence, new files will be generated only when there are actual changes in
>>> the tables.
>>
>> You lost me in those 3 sentences. Perhaps rephrasing and adding examples
>> wold make it readable. (aka what's (not)writen and when)
>
> OK I will try in v2.
>
>>
>>
>>> 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.
>>
>> what happens if an absolutely new table has been introduced which
>> is not mentioned in tests yet (will it be dumped or not)?
>
> When a new table is introduced, there is no existing data to compare the
> tables with. In this case, it will result in assertion in the following line:
>
>> g_assert(exp_sdt.aml_file);
>
> I am not sure what to do in this case except to unconditionally dump all
> tables. Maybe introduce another flag? Not sure.
Oh wait, this patch works as is since we have said in bios-tables-test.c:
> add empty files for new tables, if any, under tests/data/acpi
So for new tables the lengths will not match and the table should be dumped. I
need to test this case once.
>
>>
>>>
>>> CC: peter.mayd...@linaro.org
>>> Signed-off-by: Ani Sinha <anisi...@redhat.com>
>>> ---
>>> tests/qtest/bios-tables-test.c | 14 +++++++++++++-
>>> 1 file changed, 13 insertions(+), 1 deletion(-)
>>>
>>> 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) {
