[igt-dev] [PATCH i-g-t] test/kms_dp_dsc: Basic KMS test to validate VESA DSC on DP/eDP

[Manasi Navare]

On Wed, Oct 17, 2018 at 09:49:56AM -0700, Antonio Argenziano wrote:
> 
> 
> On 17/10/18 09:01, Manasi Navare wrote:
> >On Wed, Oct 17, 2018 at 10:48:24AM +0300, Petri Latvala wrote:
> >>On Tue, Oct 16, 2018 at 02:14:59PM -0700, Antonio Argenziano wrote:
> >>>>>>+		igt_assert_eq(ret, 0);
> >>>>>>+	}
> >>>>>>+
> >>>>>>+	close(drm_fd);
> >>>>>>+
> >>>>>>+	igt_assert_eq(ret, 0);
> >>>>>
> >>>>>'ret' is already checked in the subtest also, this would fail if you don't
> >>>>>run the subtest.
> >>>>
> >>>>If I dont run the subtest, it will use the initialized value of ret which is 0.
> >>>>It will only fail if update_display returns a negative value.
> >>>
> >>>Sorry, you are right I misread that. But, it will not be this assert to fire
> >>>if ret != 0 but the one in the subtest. My point was that this statement
> >>>seems a bit redundant and could be eliminated.
> >>
> >>It's not only redundant, it's invalid to call igt_assert when not
> >>inside an igt_fixture or a subtest. See lib/igt_core.c:1180,
> >>igt_can_fail().
> >>
> >
> >Thanks Antonio and Petri for pointing this out.
> >I will remove the igt_assert from there. The igt_fixture() should already take care
> >of having that after the test right? So I dont need to call it explicitly in igt_fixture()
> 
> You want to call asserts mainly inside the test itself, close(drm_fd) should
> probably go in a fixture at the end to avoid calling it when listing
> subtests only.
>

Ok, got it! So I can call it in update_display which essentially is the test.

Manasi
 
> Antonio
> 
> >
> >Manasi
> >>
> >>
> >>-- 
> >>Petri Latvala