Re: [PATCH 2/2] automation: add a smoke test for xen.efi on X86

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 2 Oct 2024, Stefano Stabellini wrote:
> On Thu, 3 Oct 2024, Marek Marczykowski-Górecki wrote:
> > On Wed, Oct 02, 2024 at 04:30:25PM -0700, Stefano Stabellini wrote:
> > > On Thu, 3 Oct 2024, Marek Marczykowski-Górecki wrote:
> > > > The problem is this doesn't work. The group-level variable overrides the
> > > > one in yaml. See the commit message and the link there...
> > > 
> > > Now I understand the problem, well spotted, thanks!
> > > 
> > > The idea behind having TEST_TIMEOUT defined as a project CI/CD variable
> > > is that it depends on the test infrastructure rather than the Xen code.
> > > So if we suddenly had slower runners we could change TEST_TIMEOUT
> > > without having to change the Xen code itself. So I think we should keep
> > > TEST_TIMEOUT as a project CI/CD variable.
> > > 
> > > I am not a fan of overwriting the TEST_TIMEOUT variable in the test
> > > scripts, because one test script can be used for multiple different
> > > tests, possibly even with different runners. For instance
> > > qubes-x86-64.sh works with a couple of different hardware runners that
> > > could have different timeout values. But I think it would work OK for
> > > now for our hardware-based tests (e.g. xilinx-smoke-dom0less-arm64.sh
> > > and qubes-x86-64.sh could overwrite TEST_TIMEOUT).
> > > 
> > > For this specific XTF test, I am not sure it is worth optimizing the
> > > timeout, maybe we should leave it as default. 
> > 
> > The default of 25min is quite wasteful for XTF test that failed...
> > 
> > > However if we wanted to
> > > lower the timeout value, overwriting it the way you did is OKish as I
> > > cannot think of another way.
> > 
> > If we'd need this option more often, Maybe we could set
> > TEST_TIMEOUT_OVERRIDE in test yaml, and then test script use that (if
> > present) instead? Or maybe have few "classes" of timeouts set globally
> > (TEST_TIMEOUT_SHORT, TEST_TIMEOUT_MEDIUM, TEST_TIMEOUT_LONG? or some
> > better named categories). But I don't think it's worth it for this XTF
> > test yet.
> 
> Agreed, and good idea about TEST_TIMEOUT_OVERRIDE

I decided to send a patch to implement it as I didn't want to keep the
TEST_TIMEOUT as is (ignored) for the Xilinx jobs

https://marc.info/?l=xen-devel&m=172798685928204


You should be able to use TEST_TIMEOUT_OVERRIDE in your test.yaml for
this test