diff mbox series

[v2,4/7] tests/avocado: drop malta yamon tests

Message ID 20231201093633.2551497-5-alex.bennee@linaro.org
State New
Headers show
Series final fixes for 8.2 | expand

Commit Message

Alex Bennée Dec. 1, 2023, 9:36 a.m. UTC
The assets are no longer available on the website so these are
blocking CI.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/avocado/machine_mips_malta.py | 37 -----------------------------
 1 file changed, 37 deletions(-)

Comments

Philippe Mathieu-Daudé Dec. 1, 2023, 10:24 a.m. UTC | #1
Hi Alex,

On 1/12/23 10:36, Alex Bennée wrote:
> The assets are no longer available on the website so these are
> blocking CI.
> 

How are these "blocking CI"? Missing artifact wasn't be fatal,
is it now? Also, did the artifact cache got flushed? These tests
pass locally, I disagree with removing them.

I can send a patch using YAMON_PATH like we have with RESCUE_YL_PATH,
but I still consider missing artifact shouldn't be an issue. We are
missing the point of the Avocado cache and the possibility to manually
add artifacts.

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   tests/avocado/machine_mips_malta.py | 37 -----------------------------
>   1 file changed, 37 deletions(-)
> 
> diff --git a/tests/avocado/machine_mips_malta.py b/tests/avocado/machine_mips_malta.py
> index 959dcf5602..3a1ec85c20 100644
> --- a/tests/avocado/machine_mips_malta.py
> +++ b/tests/avocado/machine_mips_malta.py
> @@ -122,40 +122,3 @@ def test_mips_malta_i6400_framebuffer_logo_8cores(self):
>           :avocado: tags=mips:smp
>           """
>           self.do_test_i6400_framebuffer_logo(8)
> -
> -class MaltaMachine(QemuSystemTest):
> -
> -    def do_test_yamon(self):
> -        rom_url = ('http://www.imgtec.com/tools/mips-tools/downloads/'
> -                   'yamon/yamon-bin-02.22.zip')
> -        rom_hash = '8da7ecddbc5312704b8b324341ee238189bde480'
> -        zip_path = self.fetch_asset(rom_url, asset_hash=rom_hash)
> -
> -        archive.extract(zip_path, self.workdir)
> -        yamon_path = os.path.join(self.workdir, 'yamon-02.22.bin')
> -
> -        self.vm.set_console()
> -        self.vm.add_args('-bios', yamon_path)
> -        self.vm.launch()
> -
> -        prompt =  'YAMON>'
> -        pattern = 'YAMON ROM Monitor'
> -        interrupt_interactive_console_until_pattern(self, pattern, prompt)
> -        wait_for_console_pattern(self, prompt)
> -        self.vm.shutdown()
> -
> -    def test_mipsel_malta_yamon(self):
> -        """
> -        :avocado: tags=arch:mipsel
> -        :avocado: tags=machine:malta
> -        :avocado: tags=endian:little
> -        """
> -        self.do_test_yamon()
> -
> -    def test_mips64el_malta_yamon(self):
> -        """
> -        :avocado: tags=arch:mips64el
> -        :avocado: tags=machine:malta
> -        :avocado: tags=endian:little
> -        """
> -        self.do_test_yamon()
Alex Bennée Dec. 1, 2023, 12:49 p.m. UTC | #2
Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Hi Alex,
>
> On 1/12/23 10:36, Alex Bennée wrote:
>> The assets are no longer available on the website so these are
>> blocking CI.
>> 
>
> How are these "blocking CI"? Missing artifact wasn't be fatal,
> is it now? Also, did the artifact cache got flushed? These tests
> pass locally, I disagree with removing them.
>
> I can send a patch using YAMON_PATH like we have with RESCUE_YL_PATH,
> but I still consider missing artifact shouldn't be an issue. We are
> missing the point of the Avocado cache and the possibility to manually
> add artifacts.

Sure but in this case the binaries are gone, you can't share them and no
one else can ever run the test. At that point it just becomes dead
weight in the repository. This doesn't stop you keeping your own branch
where old tests live on while close to a warm cache but it does somewhat
limit the use of the test to the wider community.

>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   tests/avocado/machine_mips_malta.py | 37 -----------------------------
>>   1 file changed, 37 deletions(-)
>> diff --git a/tests/avocado/machine_mips_malta.py
>> b/tests/avocado/machine_mips_malta.py
>> index 959dcf5602..3a1ec85c20 100644
>> --- a/tests/avocado/machine_mips_malta.py
>> +++ b/tests/avocado/machine_mips_malta.py
>> @@ -122,40 +122,3 @@ def test_mips_malta_i6400_framebuffer_logo_8cores(self):
>>           :avocado: tags=mips:smp
>>           """
>>           self.do_test_i6400_framebuffer_logo(8)
>> -
>> -class MaltaMachine(QemuSystemTest):
>> -
>> -    def do_test_yamon(self):
>> -        rom_url = ('http://www.imgtec.com/tools/mips-tools/downloads/'
>> -                   'yamon/yamon-bin-02.22.zip')
>> -        rom_hash = '8da7ecddbc5312704b8b324341ee238189bde480'
>> -        zip_path = self.fetch_asset(rom_url, asset_hash=rom_hash)
>> -
>> -        archive.extract(zip_path, self.workdir)
>> -        yamon_path = os.path.join(self.workdir, 'yamon-02.22.bin')
>> -
>> -        self.vm.set_console()
>> -        self.vm.add_args('-bios', yamon_path)
>> -        self.vm.launch()
>> -
>> -        prompt =  'YAMON>'
>> -        pattern = 'YAMON ROM Monitor'
>> -        interrupt_interactive_console_until_pattern(self, pattern, prompt)
>> -        wait_for_console_pattern(self, prompt)
>> -        self.vm.shutdown()
>> -
>> -    def test_mipsel_malta_yamon(self):
>> -        """
>> -        :avocado: tags=arch:mipsel
>> -        :avocado: tags=machine:malta
>> -        :avocado: tags=endian:little
>> -        """
>> -        self.do_test_yamon()
>> -
>> -    def test_mips64el_malta_yamon(self):
>> -        """
>> -        :avocado: tags=arch:mips64el
>> -        :avocado: tags=machine:malta
>> -        :avocado: tags=endian:little
>> -        """
>> -        self.do_test_yamon()
Philippe Mathieu-Daudé Dec. 1, 2023, 1:22 p.m. UTC | #3
Hi Alex,

On 1/12/23 13:49, Alex Bennée wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
>> Hi Alex,
>>
>> On 1/12/23 10:36, Alex Bennée wrote:
>>> The assets are no longer available on the website so these are
>>> blocking CI.
>>>
>>
>> How are these "blocking CI"? Missing artifact wasn't be fatal,
>> is it now? Also, did the artifact cache got flushed? These tests
>> pass locally, I disagree with removing them.
>>
>> I can send a patch using YAMON_PATH like we have with RESCUE_YL_PATH,
>> but I still consider missing artifact shouldn't be an issue. We are
>> missing the point of the Avocado cache and the possibility to manually
>> add artifacts.
> 
> Sure but in this case the binaries are gone, you can't share them and no
> one else can ever run the test. At that point it just becomes dead
> weight in the repository. This doesn't stop you keeping your own branch
> where old tests live on while close to a warm cache but it does somewhat
> limit the use of the test to the wider community.

No, I still disagree. The tests/avocado/ directory started as a place
to share tests, not to add CI gating tests. That was discussed again
2 or 3 years ago, we even recommended to change from the "opt-out on CI"
policy to the "opt-in for Gating CI". Daniel suggested the Tiers
approach, having only Tier-1 gating.

I think the mistake is to consider all tests as gating.

Why don't you want to share QEMU tests in the QEMU repository?

We might have some misunderstanding on what tests/ is for, so let's
discuss...

Regards,

Phil.
Philippe Mathieu-Daudé Dec. 1, 2023, 2:05 p.m. UTC | #4
On 1/12/23 13:49, Alex Bennée wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
>> Hi Alex,
>>
>> On 1/12/23 10:36, Alex Bennée wrote:
>>> The assets are no longer available on the website so these are
>>> blocking CI.
>>>
>>
>> How are these "blocking CI"? Missing artifact wasn't be fatal,
>> is it now? Also, did the artifact cache got flushed? These tests
>> pass locally, I disagree with removing them.
>>
>> I can send a patch using YAMON_PATH like we have with RESCUE_YL_PATH,
>> but I still consider missing artifact shouldn't be an issue. We are
>> missing the point of the Avocado cache and the possibility to manually
>> add artifacts.
> 
> Sure but in this case the binaries are gone, you can't share them and no

Per https://mips.com/develop/tools/boot-loaders/:
"To support existing users of these, and the QEMU project, YAMON is now 
available under the GPL License."

Their link https://mips.com/downloads/yamon-version-02-22/ is incorrect,
Mark emailed the webmaster to ask for the correct one.

Mark, any update?

> one else can ever run the test. At that point it just becomes dead
> weight in the repository. This doesn't stop you keeping your own branch
> where old tests live on while close to a warm cache but it does somewhat
> limit the use of the test to the wider community.
> 
>>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>    tests/avocado/machine_mips_malta.py | 37 -----------------------------
>>>    1 file changed, 37 deletions(-)
>>> diff --git a/tests/avocado/machine_mips_malta.py
>>> b/tests/avocado/machine_mips_malta.py
>>> index 959dcf5602..3a1ec85c20 100644
>>> --- a/tests/avocado/machine_mips_malta.py
>>> +++ b/tests/avocado/machine_mips_malta.py
>>> @@ -122,40 +122,3 @@ def test_mips_malta_i6400_framebuffer_logo_8cores(self):
>>>            :avocado: tags=mips:smp
>>>            """
>>>            self.do_test_i6400_framebuffer_logo(8)
>>> -
>>> -class MaltaMachine(QemuSystemTest):
>>> -
>>> -    def do_test_yamon(self):
>>> -        rom_url = ('http://www.imgtec.com/tools/mips-tools/downloads/'
>>> -                   'yamon/yamon-bin-02.22.zip')
>>> -        rom_hash = '8da7ecddbc5312704b8b324341ee238189bde480'
>>> -        zip_path = self.fetch_asset(rom_url, asset_hash=rom_hash)
>>> -
>>> -        archive.extract(zip_path, self.workdir)
>>> -        yamon_path = os.path.join(self.workdir, 'yamon-02.22.bin')
>>> -
>>> -        self.vm.set_console()
>>> -        self.vm.add_args('-bios', yamon_path)
>>> -        self.vm.launch()
>>> -
>>> -        prompt =  'YAMON>'
>>> -        pattern = 'YAMON ROM Monitor'
>>> -        interrupt_interactive_console_until_pattern(self, pattern, prompt)
>>> -        wait_for_console_pattern(self, prompt)
>>> -        self.vm.shutdown()
>>> -
>>> -    def test_mipsel_malta_yamon(self):
>>> -        """
>>> -        :avocado: tags=arch:mipsel
>>> -        :avocado: tags=machine:malta
>>> -        :avocado: tags=endian:little
>>> -        """
>>> -        self.do_test_yamon()
>>> -
>>> -    def test_mips64el_malta_yamon(self):
>>> -        """
>>> -        :avocado: tags=arch:mips64el
>>> -        :avocado: tags=machine:malta
>>> -        :avocado: tags=endian:little
>>> -        """
>>> -        self.do_test_yamon()
>
Alex Bennée Dec. 1, 2023, 3:16 p.m. UTC | #5
Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> On 1/12/23 13:49, Alex Bennée wrote:
>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>> 
>>> Hi Alex,
>>>
>>> On 1/12/23 10:36, Alex Bennée wrote:
>>>> The assets are no longer available on the website so these are
>>>> blocking CI.
>>>>
>>>
>>> How are these "blocking CI"? Missing artifact wasn't be fatal,
>>> is it now? Also, did the artifact cache got flushed? These tests
>>> pass locally, I disagree with removing them.
>>>
>>> I can send a patch using YAMON_PATH like we have with RESCUE_YL_PATH,
>>> but I still consider missing artifact shouldn't be an issue. We are
>>> missing the point of the Avocado cache and the possibility to manually
>>> add artifacts.
>> Sure but in this case the binaries are gone, you can't share them
>> and no
>
> Per https://mips.com/develop/tools/boot-loaders/:
> "To support existing users of these, and the QEMU project, YAMON is
> now available under the GPL License."

Maybe we should build it like the other ROMs?

Anyway I've dropped this commit from the PR so I'll leave it to you to
fix up however you want.
diff mbox series

Patch

diff --git a/tests/avocado/machine_mips_malta.py b/tests/avocado/machine_mips_malta.py
index 959dcf5602..3a1ec85c20 100644
--- a/tests/avocado/machine_mips_malta.py
+++ b/tests/avocado/machine_mips_malta.py
@@ -122,40 +122,3 @@  def test_mips_malta_i6400_framebuffer_logo_8cores(self):
         :avocado: tags=mips:smp
         """
         self.do_test_i6400_framebuffer_logo(8)
-
-class MaltaMachine(QemuSystemTest):
-
-    def do_test_yamon(self):
-        rom_url = ('http://www.imgtec.com/tools/mips-tools/downloads/'
-                   'yamon/yamon-bin-02.22.zip')
-        rom_hash = '8da7ecddbc5312704b8b324341ee238189bde480'
-        zip_path = self.fetch_asset(rom_url, asset_hash=rom_hash)
-
-        archive.extract(zip_path, self.workdir)
-        yamon_path = os.path.join(self.workdir, 'yamon-02.22.bin')
-
-        self.vm.set_console()
-        self.vm.add_args('-bios', yamon_path)
-        self.vm.launch()
-
-        prompt =  'YAMON>'
-        pattern = 'YAMON ROM Monitor'
-        interrupt_interactive_console_until_pattern(self, pattern, prompt)
-        wait_for_console_pattern(self, prompt)
-        self.vm.shutdown()
-
-    def test_mipsel_malta_yamon(self):
-        """
-        :avocado: tags=arch:mipsel
-        :avocado: tags=machine:malta
-        :avocado: tags=endian:little
-        """
-        self.do_test_yamon()
-
-    def test_mips64el_malta_yamon(self):
-        """
-        :avocado: tags=arch:mips64el
-        :avocado: tags=machine:malta
-        :avocado: tags=endian:little
-        """
-        self.do_test_yamon()