Message ID | 20231201093633.2551497-5-alex.bennee@linaro.org |
---|---|
State | New |
Headers | show |
Series | final fixes for 8.2 | expand |
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()
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()
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.
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() >
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 --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()
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(-)