Re: [RFC PATCH] tests/functional: Don't fail any precaching errors

["Nicholas Piggin" <npiggin@gmail.com>]

On Tue Mar 11, 2025 at 11:37 PM AEST, Daniel P. Berrangé wrote:
> On Tue, Mar 11, 2025 at 11:13:26PM +1000, Nicholas Piggin wrote:
>> The NetBSD archive is currently failing part-way through downloads,
>> which results in no clean HTTP error but a short transfer and checksum
>> error. This is treated as fatal in the precache download, and it halts
>> an entire set of tests even if some others could run.
>> 
>> I hacked up this patch to get a bunch of CI tests going again for ppc
>> merge testing.
>> 
>> Don't treat any precaching failures as errors.
>> This causes tests to be skipped when they try to fetch their asset.
>> Some CI results before/after patching:
>> 
>> functional-system-fedora
>> https://gitlab.com/npiggin/qemu/-/jobs/9370860490 #bad
>> https://gitlab.com/npiggin/qemu/-/jobs/9373246826 #good
>> 
>> functional-system-debian
>> https://gitlab.com/npiggin/qemu/-/jobs/9370860479 #bda
>> https://gitlab.com/npiggin/qemu/-/jobs/9373246822 #good
>> 
>> This is making the tests skip. Is there a way to make the error more
>> prominent / obvious in the output? Should they fail instead? I think
>> there should be a more obvious indication of failure due to asset so
>> it does not go unnoticed.
>> 
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>>  tests/functional/qemu_test/asset.py | 9 +++------
>>  1 file changed, 3 insertions(+), 6 deletions(-)
>> 
>> diff --git a/tests/functional/qemu_test/asset.py b/tests/functional/qemu_test/asset.py
>> index f0730695f09..3134ccb10da 100644
>> --- a/tests/functional/qemu_test/asset.py
>> +++ b/tests/functional/qemu_test/asset.py
>> @@ -174,14 +174,11 @@ def precache_test(test):
>>                  try:
>>                      asset.fetch()
>>                  except HTTPError as e:
>> -                    # Treat 404 as fatal, since it is highly likely to
>> -                    # indicate a broken test rather than a transient
>> -                    # server or networking problem
>> -                    if e.code == 404:
>> -                        raise
>> -
>
> Why are you removing this ? The commit above does not make any reference
> to the problem being a missing URL (404 code). We want missing URLs to
> be fatal so that we identify when images we rely on are deleted by their
> host, as that is not a transient problem.

Yeah it was just a quick hack I guess so I didn't put a
complete changelog or get it quite to the point I wanted.

I think *no* precaching errors including 404 should cause
failure because you would still want other tests to proceed
(in some cases).

But the failure should be caught when the test case tries to
fetch the asset, so you can still easily identify the download
failure.

Thanks,
Nick