From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Kumar, Mahesh" Subject: Re: [PATCH i-g-t] tests/kms_pipe_crc_basic: expect setting bad source to fail Date: Thu, 2 Aug 2018 16:35:51 +0530 Message-ID: References: <20180702111613.8326-1-mahesh1.kumar@intel.com> <387a425a-8177-84e8-5ae9-e884c28ad3d7@linux.intel.com> <8a80db90-7b62-0242-37a8-d46515bd024a@linux.intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1045855579==" Return-path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTPS id AF1896E0E6 for ; Thu, 2 Aug 2018 11:05:55 +0000 (UTC) In-Reply-To: <8a80db90-7b62-0242-37a8-d46515bd024a@linux.intel.com> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Maarten Lankhorst , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org This is a multi-part message in MIME format. --===============1045855579== Content-Type: multipart/alternative; boundary="------------1C4A0406B244EB5E687CADC4" Content-Language: en-US This is a multi-part message in MIME format. --------------1C4A0406B244EB5E687CADC4 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Hi, On 8/2/2018 4:13 PM, Maarten Lankhorst wrote: > Op 02-08-18 om 12:42 schreef Maarten Lankhorst: >> Op 02-07-18 om 13:27 schreef Maarten Lankhorst: >>> Op 02-07-18 om 13:16 schreef Mahesh Kumar: >>>> Now crc-core framework verifies the source string passed by the user. >>>> So setting bad-source will fail. Expect file write to fail in bad-source >>>> subtest of kms_pipe_crc_basic. >>>> >>>> Signed-off-by: Mahesh Kumar >>>> --- >>>> tests/kms_pipe_crc_basic.c | 3 +-- >>>> 1 file changed, 1 insertion(+), 2 deletions(-) >>>> >>>> diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c >>>> index 235fdc38..2d4dfee8 100644 >>>> --- a/tests/kms_pipe_crc_basic.c >>>> +++ b/tests/kms_pipe_crc_basic.c >>>> @@ -48,8 +48,7 @@ static struct { >>>> >>>> static void test_bad_source(data_t *data) >>>> { >>>> - igt_assert(igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo")); >>>> - igt_assert(openat(data->debugfs, "crtc-0/crc/data", O_WRONLY) == -1); >>>> + igt_assert(!igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo")); >>>> } >>>> >>>> #define N_CRCS 3 >>> New behavior makes more sense. >>> >>> Reviewed-by: Maarten Lankhorst >>> >>> Do you have igt commit rights? >>> >> Any objections if we change this to allow both behaviors? >> >> diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c >> index 235fdc386ba2..91909fa42f2f 100644 >> --- a/tests/kms_pipe_crc_basic.c >> +++ b/tests/kms_pipe_crc_basic.c >> @@ -48,8 +48,11 @@ static struct { >> >> static void test_bad_source(data_t *data) >> { >> - igt_assert(igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo")); >> - igt_assert(openat(data->debugfs, "crtc-0/crc/data", O_WRONLY) == -1); >> + errno = 0; >> + if (igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo")) >> + igt_assert(openat(data->debugfs, "crtc-0/crc/data", O_WRONLY) == -1); >> + else >> + igt_assert_eq(errno, EINVAL); >> } >> >> #define N_CRCS 3 >> > Hm without the else, errno should be EINVAL in any case.. agree, with this change Reviewed-by: Mahesh Kumar --------------1C4A0406B244EB5E687CADC4 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: 7bit

Hi,


On 8/2/2018 4:13 PM, Maarten Lankhorst wrote:
Op 02-08-18 om 12:42 schreef Maarten Lankhorst:

Op 02-07-18 om 13:27 schreef Maarten Lankhorst:

Op 02-07-18 om 13:16 schreef Mahesh Kumar:

Now crc-core framework verifies the source string passed by the user.
So setting bad-source will fail. Expect file write to fail in bad-source
subtest of kms_pipe_crc_basic.

Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
---
 tests/kms_pipe_crc_basic.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
index 235fdc38..2d4dfee8 100644
--- a/tests/kms_pipe_crc_basic.c
+++ b/tests/kms_pipe_crc_basic.c
@@ -48,8 +48,7 @@ static struct {
 
 static void test_bad_source(data_t *data)
 {
-	igt_assert(igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo"));
-	igt_assert(openat(data->debugfs, "crtc-0/crc/data", O_WRONLY) == -1);
+	igt_assert(!igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo"));
 }
 
 #define N_CRCS	3

New behavior makes more sense.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Do you have igt commit rights?


Any objections if we change this to allow both behaviors?

diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
index 235fdc386ba2..91909fa42f2f 100644
--- a/tests/kms_pipe_crc_basic.c
+++ b/tests/kms_pipe_crc_basic.c
@@ -48,8 +48,11 @@ static struct {
 
 static void test_bad_source(data_t *data)
 {
-	igt_assert(igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo"));
-	igt_assert(openat(data->debugfs, "crtc-0/crc/data", O_WRONLY) == -1);
+	errno = 0;
+	if (igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo"))
+		igt_assert(openat(data->debugfs, "crtc-0/crc/data", O_WRONLY) == -1);
+	else
+		igt_assert_eq(errno, EINVAL);
 }
 
 #define N_CRCS	3


Hm without the else, errno should be EINVAL in any case..
agree, with this change
Reviewed-by: Mahesh Kumar <mahesh1.kumar@intel.com> 

    

    

  


--------------1C4A0406B244EB5E687CADC4--

--===============1045855579==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: base64
Content-Disposition: inline

X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4
IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz
dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vaW50ZWwtZ2Z4Cg==

--===============1045855579==--