diff mbox

[i-g-t] tests: Mark some tests fail instead of skip

Message ID 1461246946-28851-1-git-send-email-gabriel.feceoru@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Feceoru, Gabriel April 21, 2016, 1:55 p.m. UTC
These checks may fail in runtime and will cause confusing
flipping skip/pass results.

Signed-off-by: Gabriel Feceoru <gabriel.feceoru@intel.com>
---
 tests/gem_exec_suspend.c     | 2 +-
 tests/gem_exec_whisper.c     | 2 +-
 tests/gem_reloc_overflow.c   | 2 +-
 tests/gem_ringfill.c         | 2 +-
 tests/gem_streaming_writes.c | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

Comments

Chris Wilson April 21, 2016, 2:07 p.m. UTC | #1
On Thu, Apr 21, 2016 at 04:55:46PM +0300, Gabriel Feceoru wrote:
> These checks may fail in runtime and will cause confusing
> flipping skip/pass results.
> 
> Signed-off-by: Gabriel Feceoru <gabriel.feceoru@intel.com>
> ---
>  tests/gem_exec_suspend.c     | 2 +-
>  tests/gem_exec_whisper.c     | 2 +-
>  tests/gem_reloc_overflow.c   | 2 +-
>  tests/gem_ringfill.c         | 2 +-
>  tests/gem_streaming_writes.c | 2 +-
>  5 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/gem_exec_suspend.c b/tests/gem_exec_suspend.c
> index cd133cc..1d03b4d 100644
> --- a/tests/gem_exec_suspend.c
> +++ b/tests/gem_exec_suspend.c
> @@ -154,7 +154,7 @@ static void run_test(int fd, unsigned engine, unsigned flags)
>  	obj[0].flags |= EXEC_OBJECT_WRITE;
>  	obj[1].handle = gem_create(fd, 4096);
>  	gem_write(fd, obj[1].handle, 0, &bbe, sizeof(bbe));
> -	igt_require(__gem_execbuf(fd, &execbuf) == 0);
> +	igt_assert(__gem_execbuf(fd, &execbuf) == 0);
>  	gem_close(fd, obj[1].handle);

Nope, here were are testing for kernel NORELOC support.

>  	memset(&reloc, 0, sizeof(reloc));
> diff --git a/tests/gem_exec_whisper.c b/tests/gem_exec_whisper.c
> index 8db475e..c55ee9c 100644
> --- a/tests/gem_exec_whisper.c
> +++ b/tests/gem_exec_whisper.c
> @@ -181,7 +181,7 @@ static void whisper(int fd, unsigned engine, unsigned flags)
>  		execbuf.flags |= LOCAL_I915_EXEC_NO_RELOC;
>  		if (gen < 6)
>  			execbuf.flags |= I915_EXEC_SECURE;
> -		igt_require(__gem_execbuf(fd, &execbuf) == 0);
> +		igt_assert(__gem_execbuf(fd, &execbuf) == 0);
>  		scratch = tmp[0];
>  		store = tmp[1];
>  	}

Nope, here were are testing for kernel NORELOC support.

> diff --git a/tests/gem_reloc_overflow.c b/tests/gem_reloc_overflow.c
> index d60bec9..dbff126 100644
> --- a/tests/gem_reloc_overflow.c
> +++ b/tests/gem_reloc_overflow.c
> @@ -208,7 +208,7 @@ static void reloc_tests(const char *suffix)
>  
>  		/* Make sure the batch would succeed except for the thing we're
>  		 * testing. */
> -		igt_require(__gem_execbuf(fd, &execbuf) == 0);
> +		igt_assert(__gem_execbuf(fd, &execbuf) == 0);

This one! This one looks ok to be assert.

>  	igt_subtest_f("batch-start-unaligned%s", suffix) {
> diff --git a/tests/gem_ringfill.c b/tests/gem_ringfill.c
> index f2dc803..1f53aac 100644
> --- a/tests/gem_ringfill.c
> +++ b/tests/gem_ringfill.c
> @@ -113,7 +113,7 @@ static void run_test(int fd, unsigned ring, unsigned flags)
>  	obj[0].flags |= EXEC_OBJECT_WRITE;
>  	obj[1].handle = gem_create(fd, 1024*16 + 4096);
>  	gem_write(fd, obj[1].handle, 0, &bbe, sizeof(bbe));
> -	igt_require(__gem_execbuf(fd, &execbuf) == 0);
> +	igt_assert(__gem_execbuf(fd, &execbuf) == 0);

Nope, here were are testing for kernel NORELOC support.

>  	obj[1].relocs_ptr = (uintptr_t)reloc;
>  	obj[1].relocation_count = 1024;
> diff --git a/tests/gem_streaming_writes.c b/tests/gem_streaming_writes.c
> index 5d0014a..d084454 100644
> --- a/tests/gem_streaming_writes.c
> +++ b/tests/gem_streaming_writes.c
> @@ -106,7 +106,7 @@ static void test_streaming(int fd, int mode, int sync)
>  	execbuf.flags = LOCAL_I915_EXEC_HANDLE_LUT;
>  	if (__gem_execbuf(fd, &execbuf)) {
>  		execbuf.flags = 0;
> -		igt_require(__gem_execbuf(fd, &execbuf) == 0);
> +		igt_assert(__gem_execbuf(fd, &execbuf) == 0);

Nope, here were are testing for kernel NORELOC support.

Flip/flop in these should be showing up as CI fail, but that is both
correct and no reason to drop backwards compatibility?
-Chris
Daniel Vetter April 21, 2016, 2:54 p.m. UTC | #2
On Thu, Apr 21, 2016 at 03:07:47PM +0100, Chris Wilson wrote:
> On Thu, Apr 21, 2016 at 04:55:46PM +0300, Gabriel Feceoru wrote:
> > These checks may fail in runtime and will cause confusing
> > flipping skip/pass results.
> > 
> > Signed-off-by: Gabriel Feceoru <gabriel.feceoru@intel.com>
> > ---
> >  tests/gem_exec_suspend.c     | 2 +-
> >  tests/gem_exec_whisper.c     | 2 +-
> >  tests/gem_reloc_overflow.c   | 2 +-
> >  tests/gem_ringfill.c         | 2 +-
> >  tests/gem_streaming_writes.c | 2 +-
> >  5 files changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tests/gem_exec_suspend.c b/tests/gem_exec_suspend.c
> > index cd133cc..1d03b4d 100644
> > --- a/tests/gem_exec_suspend.c
> > +++ b/tests/gem_exec_suspend.c
> > @@ -154,7 +154,7 @@ static void run_test(int fd, unsigned engine, unsigned flags)
> >  	obj[0].flags |= EXEC_OBJECT_WRITE;
> >  	obj[1].handle = gem_create(fd, 4096);
> >  	gem_write(fd, obj[1].handle, 0, &bbe, sizeof(bbe));
> > -	igt_require(__gem_execbuf(fd, &execbuf) == 0);
> > +	igt_assert(__gem_execbuf(fd, &execbuf) == 0);
> >  	gem_close(fd, obj[1].handle);
> 
> Nope, here were are testing for kernel NORELOC support.

For these cases, care to do a new patch to convert these to

	igt_assert_f(...,
		     "Kernel has no NORELOC support\n");

Would avoid confusion in the future.

Thanks, Daniel

> 
> >  	memset(&reloc, 0, sizeof(reloc));
> > diff --git a/tests/gem_exec_whisper.c b/tests/gem_exec_whisper.c
> > index 8db475e..c55ee9c 100644
> > --- a/tests/gem_exec_whisper.c
> > +++ b/tests/gem_exec_whisper.c
> > @@ -181,7 +181,7 @@ static void whisper(int fd, unsigned engine, unsigned flags)
> >  		execbuf.flags |= LOCAL_I915_EXEC_NO_RELOC;
> >  		if (gen < 6)
> >  			execbuf.flags |= I915_EXEC_SECURE;
> > -		igt_require(__gem_execbuf(fd, &execbuf) == 0);
> > +		igt_assert(__gem_execbuf(fd, &execbuf) == 0);
> >  		scratch = tmp[0];
> >  		store = tmp[1];
> >  	}
> 
> Nope, here were are testing for kernel NORELOC support.
> 
> > diff --git a/tests/gem_reloc_overflow.c b/tests/gem_reloc_overflow.c
> > index d60bec9..dbff126 100644
> > --- a/tests/gem_reloc_overflow.c
> > +++ b/tests/gem_reloc_overflow.c
> > @@ -208,7 +208,7 @@ static void reloc_tests(const char *suffix)
> >  
> >  		/* Make sure the batch would succeed except for the thing we're
> >  		 * testing. */
> > -		igt_require(__gem_execbuf(fd, &execbuf) == 0);
> > +		igt_assert(__gem_execbuf(fd, &execbuf) == 0);
> 
> This one! This one looks ok to be assert.
> 
> >  	igt_subtest_f("batch-start-unaligned%s", suffix) {
> > diff --git a/tests/gem_ringfill.c b/tests/gem_ringfill.c
> > index f2dc803..1f53aac 100644
> > --- a/tests/gem_ringfill.c
> > +++ b/tests/gem_ringfill.c
> > @@ -113,7 +113,7 @@ static void run_test(int fd, unsigned ring, unsigned flags)
> >  	obj[0].flags |= EXEC_OBJECT_WRITE;
> >  	obj[1].handle = gem_create(fd, 1024*16 + 4096);
> >  	gem_write(fd, obj[1].handle, 0, &bbe, sizeof(bbe));
> > -	igt_require(__gem_execbuf(fd, &execbuf) == 0);
> > +	igt_assert(__gem_execbuf(fd, &execbuf) == 0);
> 
> Nope, here were are testing for kernel NORELOC support.
> 
> >  	obj[1].relocs_ptr = (uintptr_t)reloc;
> >  	obj[1].relocation_count = 1024;
> > diff --git a/tests/gem_streaming_writes.c b/tests/gem_streaming_writes.c
> > index 5d0014a..d084454 100644
> > --- a/tests/gem_streaming_writes.c
> > +++ b/tests/gem_streaming_writes.c
> > @@ -106,7 +106,7 @@ static void test_streaming(int fd, int mode, int sync)
> >  	execbuf.flags = LOCAL_I915_EXEC_HANDLE_LUT;
> >  	if (__gem_execbuf(fd, &execbuf)) {
> >  		execbuf.flags = 0;
> > -		igt_require(__gem_execbuf(fd, &execbuf) == 0);
> > +		igt_assert(__gem_execbuf(fd, &execbuf) == 0);
> 
> Nope, here were are testing for kernel NORELOC support.
> 
> Flip/flop in these should be showing up as CI fail, but that is both
> correct and no reason to drop backwards compatibility?
> -Chris
> 
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/tests/gem_exec_suspend.c b/tests/gem_exec_suspend.c
index cd133cc..1d03b4d 100644
--- a/tests/gem_exec_suspend.c
+++ b/tests/gem_exec_suspend.c
@@ -154,7 +154,7 @@  static void run_test(int fd, unsigned engine, unsigned flags)
 	obj[0].flags |= EXEC_OBJECT_WRITE;
 	obj[1].handle = gem_create(fd, 4096);
 	gem_write(fd, obj[1].handle, 0, &bbe, sizeof(bbe));
-	igt_require(__gem_execbuf(fd, &execbuf) == 0);
+	igt_assert(__gem_execbuf(fd, &execbuf) == 0);
 	gem_close(fd, obj[1].handle);
 
 	memset(&reloc, 0, sizeof(reloc));
diff --git a/tests/gem_exec_whisper.c b/tests/gem_exec_whisper.c
index 8db475e..c55ee9c 100644
--- a/tests/gem_exec_whisper.c
+++ b/tests/gem_exec_whisper.c
@@ -181,7 +181,7 @@  static void whisper(int fd, unsigned engine, unsigned flags)
 		execbuf.flags |= LOCAL_I915_EXEC_NO_RELOC;
 		if (gen < 6)
 			execbuf.flags |= I915_EXEC_SECURE;
-		igt_require(__gem_execbuf(fd, &execbuf) == 0);
+		igt_assert(__gem_execbuf(fd, &execbuf) == 0);
 		scratch = tmp[0];
 		store = tmp[1];
 	}
diff --git a/tests/gem_reloc_overflow.c b/tests/gem_reloc_overflow.c
index d60bec9..dbff126 100644
--- a/tests/gem_reloc_overflow.c
+++ b/tests/gem_reloc_overflow.c
@@ -208,7 +208,7 @@  static void reloc_tests(const char *suffix)
 
 		/* Make sure the batch would succeed except for the thing we're
 		 * testing. */
-		igt_require(__gem_execbuf(fd, &execbuf) == 0);
+		igt_assert(__gem_execbuf(fd, &execbuf) == 0);
 	}
 
 	igt_subtest_f("batch-start-unaligned%s", suffix) {
diff --git a/tests/gem_ringfill.c b/tests/gem_ringfill.c
index f2dc803..1f53aac 100644
--- a/tests/gem_ringfill.c
+++ b/tests/gem_ringfill.c
@@ -113,7 +113,7 @@  static void run_test(int fd, unsigned ring, unsigned flags)
 	obj[0].flags |= EXEC_OBJECT_WRITE;
 	obj[1].handle = gem_create(fd, 1024*16 + 4096);
 	gem_write(fd, obj[1].handle, 0, &bbe, sizeof(bbe));
-	igt_require(__gem_execbuf(fd, &execbuf) == 0);
+	igt_assert(__gem_execbuf(fd, &execbuf) == 0);
 
 	obj[1].relocs_ptr = (uintptr_t)reloc;
 	obj[1].relocation_count = 1024;
diff --git a/tests/gem_streaming_writes.c b/tests/gem_streaming_writes.c
index 5d0014a..d084454 100644
--- a/tests/gem_streaming_writes.c
+++ b/tests/gem_streaming_writes.c
@@ -106,7 +106,7 @@  static void test_streaming(int fd, int mode, int sync)
 	execbuf.flags = LOCAL_I915_EXEC_HANDLE_LUT;
 	if (__gem_execbuf(fd, &execbuf)) {
 		execbuf.flags = 0;
-		igt_require(__gem_execbuf(fd, &execbuf) == 0);
+		igt_assert(__gem_execbuf(fd, &execbuf) == 0);
 	}
 	/* We assume that the active objects are fixed to avoid relocations */
 	__src_offset = src_offset;