[PATCH i-g-t v2 1/1] tests/intel/xe_exec_sip_eudebug: skip eudebug tests on VF

[Kamil Konieczny]

Hi,
On 2025-04-09 at 11:08:38 +0200, Manszewski, Christoph wrote:
> Hi Jan,
> 
> On 3.04.2025 13:53, Jan Sokolowski wrote:
> > Tests using eudebug are not compatible with VF functionality.
> > A way to check for that scenario is to check eudebug toggle
> > capability.
> > 
> > Skip a test that tries to use eudebug toggle if it doesn't
> > exist.
> 
> I think that the commit message is not quite appropriate. The title should
> be something like:
> 'lib/xe/xe_eudebug: Skip when disabling eudebug with no eudebug support.'

Small nit - no dot '.' at end of subject, so
s/support\./support/

> 
> And the description then should somehow capture what we have currently and
> what this change brings.
> 
> > 
> > v2: Changed the idea from disabling a single test to disabling
> > all possible tests.
> 
> Personally I am not a fan of having versions in a commit message. IMO it
> should describe the commit itself. Cover letters are a better place to track
> something like this for the sake of discussion. Ofc this is simple change so
> just skipping the version history and cover letter should be ok.
> 

Or just place versioning after '---' so git am will ignore it
and it will not be added at merge.

Sometimes they bring some info about why/what was changes
but I agree they are usally optional.

Regards,
Kamil

> > 
> > Signed-off-by: Jan Sokolowski <jan.sokolowski at intel.com>
> > Cc: Katarzyna Piecielska <katarzyna.piecielska at intel.com>
> > Reviewed-by: Lukasz Laguna <lukasz.laguna at intel.com>
> > Cc: Lukasz Laguna <lukasz.laguna at intel.com>
> > Reviewed-by: Christoph Manszewski <christoph.manszewski at intel.com>
> 
> Please don't include r-b without explicit consent.
> 
> Regards,
> Christoph
> 
> > Cc: Christoph Manszewski <christoph.manszewski at intel.com>
> > ---
> >   lib/xe/xe_eudebug.c | 5 +----
> >   1 file changed, 1 insertion(+), 4 deletions(-)
> > 
> > diff --git a/lib/xe/xe_eudebug.c b/lib/xe/xe_eudebug.c
> > index ec946058b..667c31c6e 100644
> > --- a/lib/xe/xe_eudebug.c
> > +++ b/lib/xe/xe_eudebug.c
> > @@ -1794,10 +1794,7 @@ bool xe_eudebug_enable(int fd, bool enable)
> >   	bool old = false;
> >   	int ret = __xe_eudebug_enable_getset(fd, &old, &enable);
> > -	if (ret) {
> > -		igt_skip_on(enable);
> > -		old = false;
> > -	}
> > +	igt_skip_on(ret);
> >   	return old;
> >   }
>