[igt-dev] [PATCH i-g-t] tests/kms_getfb: Use fixtures and subtest groups

[igt-dev] [PATCH i-g-t] tests/kms_getfb: Use fixtures and subtest groups

[Daniel Stone]

Make sure we gently skip when required features (CCS, getfb2) are not
supported on the running kernel, and clean up properly after ourselves.

Signed-off-by: Daniel Stone <daniels@collabora.com>
---
 tests/kms_getfb.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/tests/kms_getfb.c b/tests/kms_getfb.c
index 5f0bd2a9..81d796a4 100644
--- a/tests/kms_getfb.c
+++ b/tests/kms_getfb.c
@@ -102,7 +102,7 @@ static void test_handle_input(int fd)
 		add.pitches[0] = 1024*4;
 		add.handles[0] = igt_create_bo_with_dimensions(fd, 1024, 1024,
 			DRM_FORMAT_XRGB8888, 0, 0, NULL, NULL, NULL);
-		igt_assert(add.handles[0]);
+		igt_require(add.handles[0] != 0);
 		do_ioctl(fd, DRM_IOCTL_MODE_ADDFB2, &add);
 	}
 
@@ -134,6 +134,9 @@ static void test_handle_input(int fd)
 		igt_require(get.fb_id > 0);
 		do_ioctl_err(fd, DRM_IOCTL_MODE_GETFB, &get, ENOENT);
 	}
+
+	igt_fixture
+		gem_close(fd, add.handles[0]);
 }
 
 static void test_duplicate_handles(int fd)
@@ -198,9 +201,11 @@ igt_main
 	igt_fixture
 		fd = drm_open_driver_master(DRIVER_ANY);
 
-	test_handle_input(fd);
+	igt_subtest_group
+		test_handle_input(fd);
 
-	test_duplicate_handles(fd);
+	igt_subtest_group
+		test_duplicate_handles(fd);
 
 	igt_fixture
 		close(fd);
-- 
2.17.0.rc1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.BAT: success for tests/kms_getfb: Use fixtures and subtest groups

[Patchwork]

== Series Details ==

Series: tests/kms_getfb: Use fixtures and subtest groups
URL   : https://patchwork.freedesktop.org/series/40960/
State : success

== Summary ==

IGT patchset tested on top of latest successful build
2cbd4ddf11b3eaf01f33d8bc2ad46411ec6c299a lib/igt_kms: Improve connector probing in igt_display_init(), v2.

with latest DRM-Tip kernel build CI_DRM_4010
335ef9849310 drm-tip: 2018y-03m-30d-09h-08m-40s UTC integration manifest

No testlist changes.

---- Known issues:

Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                fail       -> PASS       (fi-gdg-551) fdo#102575
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713
Test prime_vgem:
        Subgroup basic-fence-flip:
                pass       -> FAIL       (fi-ilk-650) fdo#104008

fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008

fi-bdw-5557u     total:285  pass:264  dwarn:0   dfail:0   fail:0   skip:21  time:431s
fi-bdw-gvtdvm    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:451s
fi-blb-e6850     total:285  pass:220  dwarn:1   dfail:0   fail:0   skip:64  time:382s
fi-bsw-n3050     total:285  pass:239  dwarn:0   dfail:0   fail:0   skip:46  time:537s
fi-bwr-2160      total:285  pass:180  dwarn:0   dfail:0   fail:0   skip:105 time:298s
fi-bxt-j4205     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:518s
fi-byt-j1900     total:285  pass:250  dwarn:0   dfail:0   fail:0   skip:35  time:521s
fi-byt-n2820     total:285  pass:246  dwarn:0   dfail:0   fail:0   skip:39  time:509s
fi-cfl-8700k     total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:412s
fi-cfl-s3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:562s
fi-cfl-u         total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:515s
fi-cnl-y3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:589s
fi-elk-e7500     total:285  pass:225  dwarn:1   dfail:0   fail:0   skip:59  time:426s
fi-gdg-551       total:285  pass:177  dwarn:0   dfail:0   fail:0   skip:108 time:320s
fi-glk-1         total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:544s
fi-hsw-4770      total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:407s
fi-ilk-650       total:285  pass:224  dwarn:0   dfail:0   fail:1   skip:60  time:421s
fi-ivb-3520m     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:472s
fi-ivb-3770      total:285  pass:252  dwarn:0   dfail:0   fail:0   skip:33  time:433s
fi-kbl-7500u     total:285  pass:260  dwarn:1   dfail:0   fail:0   skip:24  time:473s
fi-kbl-7567u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:462s
fi-kbl-r         total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:510s
fi-pnv-d510      total:285  pass:219  dwarn:1   dfail:0   fail:0   skip:65  time:659s
fi-skl-6260u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:437s
fi-skl-6600u     total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:543s
fi-skl-6700k2    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:507s
fi-skl-6770hq    total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:500s
fi-skl-guc       total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:434s
fi-skl-gvtdvm    total:285  pass:262  dwarn:0   dfail:0   fail:0   skip:23  time:448s
fi-snb-2520m     total:242  pass:208  dwarn:0   dfail:0   fail:0   skip:33 
fi-snb-2600      total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:400s
Blacklisted hosts:
fi-cnl-psr       total:285  pass:256  dwarn:3   dfail:0   fail:0   skip:26  time:525s
fi-glk-j4005     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:493s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1212/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.IGT: warning for tests/kms_getfb: Use fixtures and subtest groups

[Patchwork]

== Series Details ==

Series: tests/kms_getfb: Use fixtures and subtest groups
URL   : https://patchwork.freedesktop.org/series/40960/
State : warning

== Summary ==

---- Possible new issues:

Test kms_atomic_transition:
        Subgroup 1x-modeset-transitions-fencing:
                pass       -> SKIP       (shard-snb)
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-indfb-draw-blt:
                pass       -> SKIP       (shard-snb)

---- Known issues:

Test kms_cursor_legacy:
        Subgroup 2x-long-flip-vs-cursor-atomic:
                pass       -> FAIL       (shard-hsw) fdo#104873
Test kms_flip:
        Subgroup 2x-flip-vs-expired-vblank-interruptible:
                fail       -> PASS       (shard-hsw) fdo#102887
        Subgroup 2x-plain-flip-ts-check:
                pass       -> FAIL       (shard-hsw) fdo#100368 +1
Test kms_plane_multiple:
        Subgroup atomic-pipe-a-tiling-x:
                pass       -> FAIL       (shard-snb) fdo#103166
Test kms_rotation_crc:
        Subgroup primary-rotation-180:
                pass       -> FAIL       (shard-snb) fdo#103925 +2
Test kms_sysfs_edid_timing:
                pass       -> WARN       (shard-apl) fdo#100047

fdo#104873 https://bugs.freedesktop.org/show_bug.cgi?id=104873
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047

shard-apl        total:3495 pass:1831 dwarn:1   dfail:0   fail:7   skip:1655 time:12856s
shard-hsw        total:3495 pass:1779 dwarn:1   dfail:0   fail:5   skip:1709 time:11405s
shard-snb        total:3495 pass:1370 dwarn:2   dfail:0   fail:4   skip:2119 time:7035s
Blacklisted hosts:
shard-kbl        total:3495 pass:1958 dwarn:1   dfail:0   fail:7   skip:1529 time:9306s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1212/shards.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] ✗ Fi.CI.IGT: warning for tests/kms_getfb: Use fixtures and subtest groups

[Daniel Stone]

[-- Attachment #1: Type: text/html, Size: 1234 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t] tests/kms_getfb: Use fixtures and subtest groups

[Antonio Argenziano]

On 30/03/18 08:49, Daniel Stone wrote:
> Make sure we gently skip when required features (CCS, getfb2) are not
> supported on the running kernel, and clean up properly after ourselves.
> 
> Signed-off-by: Daniel Stone <daniels@collabora.com>
> ---
>   tests/kms_getfb.c | 11 ++++++++---
>   1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/kms_getfb.c b/tests/kms_getfb.c
> index 5f0bd2a9..81d796a4 100644
> --- a/tests/kms_getfb.c
> +++ b/tests/kms_getfb.c
> @@ -102,7 +102,7 @@ static void test_handle_input(int fd)
>   		add.pitches[0] = 1024*4;
>   		add.handles[0] = igt_create_bo_with_dimensions(fd, 1024, 1024,
>   			DRM_FORMAT_XRGB8888, 0, 0, NULL, NULL, NULL);
> -		igt_assert(add.handles[0]);
> +		igt_require(add.handles[0] != 0);
>   		do_ioctl(fd, DRM_IOCTL_MODE_ADDFB2, &add);
>   	}
>   
> @@ -134,6 +134,9 @@ static void test_handle_input(int fd)
>   		igt_require(get.fb_id > 0);
>   		do_ioctl_err(fd, DRM_IOCTL_MODE_GETFB, &get, ENOENT);
>   	}
> +
> +	igt_fixture
> +		gem_close(fd, add.handles[0]);
>   }
>   
>   static void test_duplicate_handles(int fd)
> @@ -198,9 +201,11 @@ igt_main
>   	igt_fixture
>   		fd = drm_open_driver_master(DRIVER_ANY);
>   
> -	test_handle_input(fd);
> +	igt_subtest_group
> +		test_handle_input(fd);
>   
> -	test_duplicate_handles(fd);
> +	igt_subtest_group
> +		test_duplicate_handles(fd);

Since both test_handle_input and test_duplicate_handles define some 
sub-tests and some fixtures, I think it would be better to unwrap those 
functions and do all fixtures and subtest definition in igt_main to make 
it more readable. It seems like you would also be able to reuse the same 
fixture across the two subtest_groups.

FWIW I also don't think the skips in CI are caused by this change.

Thanks,
Antonio

>   
>   	igt_fixture
>   		close(fd);
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t] tests/kms_getfb: Use fixtures and subtest groups

[Daniel Stone]

Hi Antonio,

On 30 March 2018 at 22:01, Antonio Argenziano
<antonio.argenziano@intel.com> wrote:
> Since both test_handle_input and test_duplicate_handles define some
> sub-tests and some fixtures, I think it would be better to unwrap those
> functions and do all fixtures and subtest definition in igt_main to make it
> more readable. It seems like you would also be able to reuse the same
> fixture across the two subtest_groups.

To be clear, does this mean that each piece of fixture and igt_subtest
definition should live inside main?

Cheers,
Daniel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t] tests/kms_getfb: Use fixtures and subtest groups

[Antonio Argenziano]

On 02/04/18 06:02, Daniel Stone wrote:
> Hi Antonio,
> 
> On 30 March 2018 at 22:01, Antonio Argenziano
> <antonio.argenziano@intel.com> wrote:
>> Since both test_handle_input and test_duplicate_handles define some
>> sub-tests and some fixtures, I think it would be better to unwrap those
>> functions and do all fixtures and subtest definition in igt_main to make it
>> more readable. It seems like you would also be able to reuse the same
>> fixture across the two subtest_groups.
> 
> To be clear, does this mean that each piece of fixture and igt_subtest
> definition should live inside main?

I don't think there is a rule for that, it looks better that way 
(fixtures/igt_subtest/subtest_group all in the main) IMO :). Also it is 
what I've found in most if not all tests I've looked at.

Still the patch makes sense with or without my suggestion.
Acked-by: Antonio Argenziano <antonio.argenziano@intel.com>

Thanks,
Antonio

> 
> Cheers,
> Daniel
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t] tests/kms_getfb: Use fixtures and subtest groups

[Arkadiusz Hiler]

On Mon, Apr 02, 2018 at 10:13:19AM -0700, Antonio Argenziano wrote:
> 
> 
> On 02/04/18 06:02, Daniel Stone wrote:
> > Hi Antonio,
> > 
> > On 30 March 2018 at 22:01, Antonio Argenziano
> > <antonio.argenziano@intel.com> wrote:
> > > Since both test_handle_input and test_duplicate_handles define some
> > > sub-tests and some fixtures, I think it would be better to unwrap those
> > > functions and do all fixtures and subtest definition in igt_main to make it
> > > more readable. It seems like you would also be able to reuse the same
> > > fixture across the two subtest_groups.
> > 
> > To be clear, does this mean that each piece of fixture and igt_subtest
> > definition should live inside main?
> 
> I don't think there is a rule for that, it looks better that way
> (fixtures/igt_subtest/subtest_group all in the main) IMO :). Also it is what
> I've found in most if not all tests I've looked at.

Definately not a rule, and definately not a patch to enforce such
preference :-P

But generally it's a good default.

> Still the patch makes sense with or without my suggestion.
> Acked-by: Antonio Argenziano <antonio.argenziano@intel.com>

Thanks, will push in a second.
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t] tests/kms_getfb: Use fixtures and subtest groups

[Daniel Stone]

Hi Arkadiusz,

On 4 April 2018 at 15:00, Arkadiusz Hiler <arkadiusz.hiler@intel.com> wrote:
> On Mon, Apr 02, 2018 at 10:13:19AM -0700, Antonio Argenziano wrote:
>> On 02/04/18 06:02, Daniel Stone wrote:
>> > To be clear, does this mean that each piece of fixture and igt_subtest
>> > definition should live inside main?
>>
>> I don't think there is a rule for that, it looks better that way
>> (fixtures/igt_subtest/subtest_group all in the main) IMO :). Also it is what
>> I've found in most if not all tests I've looked at.
>
> Definately not a rule, and definately not a patch to enforce such
> preference :-P
>
> But generally it's a good default.

Thanks for pushing this. I haven't done the rework yet because I've
been busy with some more urgent fires this week; I'll rewrite
everything to be directly under main() later this week.

Cheers,
Daniel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t] tests/kms_getfb: Use fixtures and subtest groups

[Daniel Vetter]

On Mon, Apr 2, 2018 at 7:13 PM, Antonio Argenziano
<antonio.argenziano@intel.com> wrote:
>
>
> On 02/04/18 06:02, Daniel Stone wrote:
>>
>> Hi Antonio,
>>
>> On 30 March 2018 at 22:01, Antonio Argenziano
>> <antonio.argenziano@intel.com> wrote:
>>>
>>> Since both test_handle_input and test_duplicate_handles define some
>>> sub-tests and some fixtures, I think it would be better to unwrap those
>>> functions and do all fixtures and subtest definition in igt_main to make
>>> it
>>> more readable. It seems like you would also be able to reuse the same
>>> fixture across the two subtest_groups.
>>
>>
>> To be clear, does this mean that each piece of fixture and igt_subtest
>> definition should live inside main?
>
>
> I don't think there is a rule for that, it looks better that way
> (fixtures/igt_subtest/subtest_group all in the main) IMO :). Also it is what
> I've found in most if not all tests I've looked at.
>
> Still the patch makes sense with or without my suggestion.
> Acked-by: Antonio Argenziano <antonio.argenziano@intel.com>

igt_subtest and igt_fixture in functions, to group stuff, and then
having an overall igt_subtest_group seems a perfectly fine pattern to
me. Especially with simple tests that just check tons of uapi
combinations for input validation on the kernel side igt_main
otherwise gets extermely unwieldy.

I think this here is perfectly fine as-is.

Cheers, Daniel

>
> Thanks,
> Antonio
>
>
>>
>> Cheers,
>> Daniel
>>
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t] tests/kms_getfb: Use fixtures and subtest groups

[Arkadiusz Hiler]

On Wed, Apr 04, 2018 at 05:28:13PM +0100, Daniel Stone wrote:
> Hi Arkadiusz,
> 
> On 4 April 2018 at 15:00, Arkadiusz Hiler <arkadiusz.hiler@intel.com> wrote:
> > On Mon, Apr 02, 2018 at 10:13:19AM -0700, Antonio Argenziano wrote:
> >> On 02/04/18 06:02, Daniel Stone wrote:
> >> > To be clear, does this mean that each piece of fixture and igt_subtest
> >> > definition should live inside main?
> >>
> >> I don't think there is a rule for that, it looks better that way
> >> (fixtures/igt_subtest/subtest_group all in the main) IMO :). Also it is what
> >> I've found in most if not all tests I've looked at.
> >
> > Definately not a rule, and definately not a patch to enforce such
> > preference :-P
> >
> > But generally it's a good default.
> 
> Thanks for pushing this. I haven't done the rework yet because I've
> been busy with some more urgent fires this week; I'll rewrite
> everything to be directly under main() later this week.
> 
> Cheers,
> Daniel

There's no real need. The test is easy to follow as it is and I
personally like the grouping done this way.

As I've said - it's just a good default and not much more.

-- 
Cheers,
Arek
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev