[PATCH] tests/igt: Add runtime environment checks

[PATCH] tests/igt: Add runtime environment checks

[Daniel Vetter]

This is one of the nice pieces that I've never ported from the old
make based test runner. Note that we only use the result of the check
when actually running the testcases so that enumerating tests still
works as non-root on arbitrary machines.

Cc: Ben Widawsky <ben-Egm8mFWDmf3k1uMJSBkQmQ@public.gmane.org>
Requested-by: Ben Widawsky <ben-Egm8mFWDmf3k1uMJSBkQmQ@public.gmane.org>
Signed-off-by: Daniel Vetter <daniel.vetter-/w4YWyX8dFk@public.gmane.org>
---
 tests/igt.tests | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/tests/igt.tests b/tests/igt.tests
index f3884925deaa..8d02c1a60255 100644
--- a/tests/igt.tests
+++ b/tests/igt.tests
@@ -28,7 +28,7 @@ import sys
 import subprocess
 
 from os import path
-from framework.core import testBinDir, TestProfile
+from framework.core import testBinDir, TestProfile, TestResult
 from framework.exectest import ExecTest
 
 #############################################################################
@@ -39,6 +39,24 @@ from framework.exectest import ExecTest
 ##### automatically add all tests into the 'igt' category.
 #############################################################################
 
+def checkEnvironment():
+    debugfs_path = "/sys/kernel/debug/dri"
+    if os.getuid() != 0:
+        print "Test Environment check: not root!"
+        return False
+    if not os.path.isdir(debugfs_path):
+        print "Test Environment check: debugfs not mounted properly!"
+        return False
+    for subdir in os.listdir(debugfs_path):
+        clients = open(os.path.join(debugfs_path, subdir, "clients"), 'r')
+        lines = clients.readlines()
+        if len(lines) > 2:
+            print "Test Environment check: other drm clients running!"
+            return False
+
+    print "Test Environment check: Succeeded."
+    return True
+
 if not os.path.exists(os.path.join(testBinDir, 'igt')):
     print "igt symlink not found!"
     sys.exit(0)
@@ -46,6 +64,8 @@ if not os.path.exists(os.path.join(testBinDir, 'igt')):
 # Chase the piglit/bin/igt symlink to find where the tests really live.
 igtTestRoot = path.join(path.realpath(path.join(testBinDir, 'igt')), 'tests')
 
+igtEnvironmentOk = checkEnvironment()
+
 profile = TestProfile()
 
 class IGTTest(ExecTest):
@@ -54,6 +74,9 @@ class IGTTest(ExecTest):
         self.timeout = 60*20 # 20 minutes deadline by default
 
     def interpretResult(self, out, returncode, results, dmesg):
+        if not igtEnvironmentOk:
+            return out
+
         if returncode == 0:
             results['result'] = 'dmesg-warn' if dmesg != '' else 'pass'
         elif returncode == 77:
@@ -63,6 +86,13 @@ class IGTTest(ExecTest):
         return out
     def run(self, env):
         env.dmesg = True
+        if not igtEnvironmentOk:
+            results = TestResult()
+            results['result'] = 'skip'
+            results['info'] = unicode("Test Environment isn't OK")
+
+            return results
+
         return ExecTest.run(self, env)
 
 def listTests(listname):
-- 
1.8.1.4

Re: [PATCH] tests/igt: Add runtime environment checks

[Ben Widawsky]

On Tue, Nov 26, 2013 at 09:09:04AM +0100, Daniel Vetter wrote:
> This is one of the nice pieces that I've never ported from the old
> make based test runner. Note that we only use the result of the check
> when actually running the testcases so that enumerating tests still
> works as non-root on arbitrary machines.
> 

So does this bail on the first test, or does this run for all tests?
BTW, Damien said he had also implemented this.

> Cc: Ben Widawsky <ben@bwidawsk.net>
> Requested-by: Ben Widawsky <ben@bwidawsk.net>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  tests/igt.tests | 32 +++++++++++++++++++++++++++++++-
>  1 file changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/igt.tests b/tests/igt.tests
> index f3884925deaa..8d02c1a60255 100644
> --- a/tests/igt.tests
> +++ b/tests/igt.tests
> @@ -28,7 +28,7 @@ import sys
>  import subprocess
>  
>  from os import path
> -from framework.core import testBinDir, TestProfile
> +from framework.core import testBinDir, TestProfile, TestResult
>  from framework.exectest import ExecTest
>  
>  #############################################################################
> @@ -39,6 +39,24 @@ from framework.exectest import ExecTest
>  ##### automatically add all tests into the 'igt' category.
>  #############################################################################
>  
> +def checkEnvironment():
> +    debugfs_path = "/sys/kernel/debug/dri"
> +    if os.getuid() != 0:
> +        print "Test Environment check: not root!"
> +        return False
> +    if not os.path.isdir(debugfs_path):
> +        print "Test Environment check: debugfs not mounted properly!"
> +        return False
> +    for subdir in os.listdir(debugfs_path):
> +        clients = open(os.path.join(debugfs_path, subdir, "clients"), 'r')
> +        lines = clients.readlines()
> +        if len(lines) > 2:
> +            print "Test Environment check: other drm clients running!"
> +            return False
> +
> +    print "Test Environment check: Succeeded."
> +    return True
> +
>  if not os.path.exists(os.path.join(testBinDir, 'igt')):
>      print "igt symlink not found!"
>      sys.exit(0)
> @@ -46,6 +64,8 @@ if not os.path.exists(os.path.join(testBinDir, 'igt')):
>  # Chase the piglit/bin/igt symlink to find where the tests really live.
>  igtTestRoot = path.join(path.realpath(path.join(testBinDir, 'igt')), 'tests')
>  
> +igtEnvironmentOk = checkEnvironment()
> +
>  profile = TestProfile()
>  
>  class IGTTest(ExecTest):
> @@ -54,6 +74,9 @@ class IGTTest(ExecTest):
>          self.timeout = 60*20 # 20 minutes deadline by default
>  
>      def interpretResult(self, out, returncode, results, dmesg):
> +        if not igtEnvironmentOk:
> +            return out
> +
>          if returncode == 0:
>              results['result'] = 'dmesg-warn' if dmesg != '' else 'pass'
>          elif returncode == 77:
> @@ -63,6 +86,13 @@ class IGTTest(ExecTest):
>          return out
>      def run(self, env):
>          env.dmesg = True
> +        if not igtEnvironmentOk:
> +            results = TestResult()
> +            results['result'] = 'skip'
> +            results['info'] = unicode("Test Environment isn't OK")
> +
> +            return results
> +
>          return ExecTest.run(self, env)
>  
>  def listTests(listname):
> -- 
> 1.8.1.4
> 

-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [PATCH] tests/igt: Add runtime environment checks

[Daniel Vetter]

On Tue, Nov 26, 2013 at 11:05:57AM -0800, Ben Widawsky wrote:
> On Tue, Nov 26, 2013 at 09:09:04AM +0100, Daniel Vetter wrote:
> > This is one of the nice pieces that I've never ported from the old
> > make based test runner. Note that we only use the result of the check
> > when actually running the testcases so that enumerating tests still
> > works as non-root on arbitrary machines.
> > 
> 
> So does this bail on the first test, or does this run for all tests?

It bails each test individually. Since otherwise enumerating them wouldn't
work any more. I've hoped the commit message was clear about it.

> BTW, Damien said he had also implemented this.

Only in the make target he created, not in piglit itself. Imo we should
have all the testrunner logic in one place, i.e. in the piglit sources.
-Daniel

> 
> > Cc: Ben Widawsky <ben-Egm8mFWDmf3k1uMJSBkQmQ@public.gmane.org>
> > Requested-by: Ben Widawsky <ben-Egm8mFWDmf3k1uMJSBkQmQ@public.gmane.org>
> > Signed-off-by: Daniel Vetter <daniel.vetter-/w4YWyX8dFk@public.gmane.org>
> > ---
> >  tests/igt.tests | 32 +++++++++++++++++++++++++++++++-
> >  1 file changed, 31 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/igt.tests b/tests/igt.tests
> > index f3884925deaa..8d02c1a60255 100644
> > --- a/tests/igt.tests
> > +++ b/tests/igt.tests
> > @@ -28,7 +28,7 @@ import sys
> >  import subprocess
> >  
> >  from os import path
> > -from framework.core import testBinDir, TestProfile
> > +from framework.core import testBinDir, TestProfile, TestResult
> >  from framework.exectest import ExecTest
> >  
> >  #############################################################################
> > @@ -39,6 +39,24 @@ from framework.exectest import ExecTest
> >  ##### automatically add all tests into the 'igt' category.
> >  #############################################################################
> >  
> > +def checkEnvironment():
> > +    debugfs_path = "/sys/kernel/debug/dri"
> > +    if os.getuid() != 0:
> > +        print "Test Environment check: not root!"
> > +        return False
> > +    if not os.path.isdir(debugfs_path):
> > +        print "Test Environment check: debugfs not mounted properly!"
> > +        return False
> > +    for subdir in os.listdir(debugfs_path):
> > +        clients = open(os.path.join(debugfs_path, subdir, "clients"), 'r')
> > +        lines = clients.readlines()
> > +        if len(lines) > 2:
> > +            print "Test Environment check: other drm clients running!"
> > +            return False
> > +
> > +    print "Test Environment check: Succeeded."
> > +    return True
> > +
> >  if not os.path.exists(os.path.join(testBinDir, 'igt')):
> >      print "igt symlink not found!"
> >      sys.exit(0)
> > @@ -46,6 +64,8 @@ if not os.path.exists(os.path.join(testBinDir, 'igt')):
> >  # Chase the piglit/bin/igt symlink to find where the tests really live.
> >  igtTestRoot = path.join(path.realpath(path.join(testBinDir, 'igt')), 'tests')
> >  
> > +igtEnvironmentOk = checkEnvironment()
> > +
> >  profile = TestProfile()
> >  
> >  class IGTTest(ExecTest):
> > @@ -54,6 +74,9 @@ class IGTTest(ExecTest):
> >          self.timeout = 60*20 # 20 minutes deadline by default
> >  
> >      def interpretResult(self, out, returncode, results, dmesg):
> > +        if not igtEnvironmentOk:
> > +            return out
> > +
> >          if returncode == 0:
> >              results['result'] = 'dmesg-warn' if dmesg != '' else 'pass'
> >          elif returncode == 77:
> > @@ -63,6 +86,13 @@ class IGTTest(ExecTest):
> >          return out
> >      def run(self, env):
> >          env.dmesg = True
> > +        if not igtEnvironmentOk:
> > +            results = TestResult()
> > +            results['result'] = 'skip'
> > +            results['info'] = unicode("Test Environment isn't OK")
> > +
> > +            return results
> > +
> >          return ExecTest.run(self, env)
> >  
> >  def listTests(listname):
> > -- 
> > 1.8.1.4
> > 
> 
> -- 
> Ben Widawsky, Intel Open Source Technology Center

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] tests/igt: Add runtime environment checks

[Ben Widawsky]

On Tue, Nov 26, 2013 at 08:46:19PM +0100, Daniel Vetter wrote:
> On Tue, Nov 26, 2013 at 11:05:57AM -0800, Ben Widawsky wrote:
> > On Tue, Nov 26, 2013 at 09:09:04AM +0100, Daniel Vetter wrote:
> > > This is one of the nice pieces that I've never ported from the old
> > > make based test runner. Note that we only use the result of the check
> > > when actually running the testcases so that enumerating tests still
> > > works as non-root on arbitrary machines.
> > > 
> > 
> > So does this bail on the first test, or does this run for all tests?
> 
> It bails each test individually. Since otherwise enumerating them wouldn't
> work any more. I've hoped the commit message was clear about it.

It wasn't clear to me, though it seemed to be the case from the diff.
I'd prefer an early bail, and just check once, and ideally, not
generating a JSON file at all.

> 
> > BTW, Damien said he had also implemented this.
> 
> Only in the make target he created, not in piglit itself. Imo we should
> have all the testrunner logic in one place, i.e. in the piglit sources.
> -Daniel

Damien, can you comment? I could have sworn you said something different
on IRC. It sounded like exactly what I wanted.

> 
> > 
> > > Cc: Ben Widawsky <ben-Egm8mFWDmf3k1uMJSBkQmQ@public.gmane.org>
> > > Requested-by: Ben Widawsky <ben-Egm8mFWDmf3k1uMJSBkQmQ@public.gmane.org>
> > > Signed-off-by: Daniel Vetter <daniel.vetter-/w4YWyX8dFk@public.gmane.org>
> > > ---
> > >  tests/igt.tests | 32 +++++++++++++++++++++++++++++++-
> > >  1 file changed, 31 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/tests/igt.tests b/tests/igt.tests
> > > index f3884925deaa..8d02c1a60255 100644
> > > --- a/tests/igt.tests
> > > +++ b/tests/igt.tests
> > > @@ -28,7 +28,7 @@ import sys
> > >  import subprocess
> > >  
> > >  from os import path
> > > -from framework.core import testBinDir, TestProfile
> > > +from framework.core import testBinDir, TestProfile, TestResult
> > >  from framework.exectest import ExecTest
> > >  
> > >  #############################################################################
> > > @@ -39,6 +39,24 @@ from framework.exectest import ExecTest
> > >  ##### automatically add all tests into the 'igt' category.
> > >  #############################################################################
> > >  
> > > +def checkEnvironment():
> > > +    debugfs_path = "/sys/kernel/debug/dri"
> > > +    if os.getuid() != 0:
> > > +        print "Test Environment check: not root!"
> > > +        return False
> > > +    if not os.path.isdir(debugfs_path):
> > > +        print "Test Environment check: debugfs not mounted properly!"
> > > +        return False
> > > +    for subdir in os.listdir(debugfs_path):
> > > +        clients = open(os.path.join(debugfs_path, subdir, "clients"), 'r')
> > > +        lines = clients.readlines()
> > > +        if len(lines) > 2:
> > > +            print "Test Environment check: other drm clients running!"
> > > +            return False
> > > +
> > > +    print "Test Environment check: Succeeded."
> > > +    return True
> > > +
> > >  if not os.path.exists(os.path.join(testBinDir, 'igt')):
> > >      print "igt symlink not found!"
> > >      sys.exit(0)
> > > @@ -46,6 +64,8 @@ if not os.path.exists(os.path.join(testBinDir, 'igt')):
> > >  # Chase the piglit/bin/igt symlink to find where the tests really live.
> > >  igtTestRoot = path.join(path.realpath(path.join(testBinDir, 'igt')), 'tests')
> > >  
> > > +igtEnvironmentOk = checkEnvironment()
> > > +
> > >  profile = TestProfile()
> > >  
> > >  class IGTTest(ExecTest):
> > > @@ -54,6 +74,9 @@ class IGTTest(ExecTest):
> > >          self.timeout = 60*20 # 20 minutes deadline by default
> > >  
> > >      def interpretResult(self, out, returncode, results, dmesg):
> > > +        if not igtEnvironmentOk:
> > > +            return out
> > > +
> > >          if returncode == 0:
> > >              results['result'] = 'dmesg-warn' if dmesg != '' else 'pass'
> > >          elif returncode == 77:
> > > @@ -63,6 +86,13 @@ class IGTTest(ExecTest):
> > >          return out
> > >      def run(self, env):
> > >          env.dmesg = True
> > > +        if not igtEnvironmentOk:
> > > +            results = TestResult()
> > > +            results['result'] = 'skip'
> > > +            results['info'] = unicode("Test Environment isn't OK")
> > > +
> > > +            return results
> > > +
> > >          return ExecTest.run(self, env)
> > >  
> > >  def listTests(listname):
> > > -- 
> > > 1.8.1.4
> > > 
> > 
> > -- 
> > Ben Widawsky, Intel Open Source Technology Center
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [PATCH] tests/igt: Add runtime environment checks

[Damien Lespiau]

On Tue, Nov 26, 2013 at 11:49:43AM -0800, Ben Widawsky wrote:
> > Only in the make target he created, not in piglit itself. Imo we should
> > have all the testrunner logic in one place, i.e. in the piglit sources.
> > -Daniel
> 
> Damien, can you comment? I could have sworn you said something different
> on IRC. It sounded like exactly what I wanted.

What Daniel says is correct, the check is part of the runner wrapper,
not piglit itself.

I'd rather have a environement check up-front and I don't mind where it
lives (igt Vs piglit).

-- 
Damien

Re: [PATCH] tests/igt: Add runtime environment checks

[Daniel Vetter]

On Tue, Nov 26, 2013 at 8:53 PM, Damien Lespiau
<damien.lespiau-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> On Tue, Nov 26, 2013 at 11:49:43AM -0800, Ben Widawsky wrote:
>> > Only in the make target he created, not in piglit itself. Imo we should
>> > have all the testrunner logic in one place, i.e. in the piglit sources.
>> > -Daniel
>>
>> Damien, can you comment? I could have sworn you said something different
>> on IRC. It sounded like exactly what I wanted.
>
> What Daniel says is correct, the check is part of the runner wrapper,
> not piglit itself.
>
> I'd rather have a environement check up-front and I don't mind where it
> lives (igt Vs piglit).

The problem is that generating the testlist (or printing the commands)
is a feature QA actually relies on. I also use it occasionally to
quickly test igt library changes. So we can't bail that early. My
patch bails fairly late, but I didn't see a better spot.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] tests/igt: Add runtime environment checks

[Ben Widawsky]

On Tue, Nov 26, 2013 at 08:55:40PM +0100, Daniel Vetter wrote:
> On Tue, Nov 26, 2013 at 8:53 PM, Damien Lespiau
> <damien.lespiau-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> > On Tue, Nov 26, 2013 at 11:49:43AM -0800, Ben Widawsky wrote:
> >> > Only in the make target he created, not in piglit itself. Imo we should
> >> > have all the testrunner logic in one place, i.e. in the piglit sources.
> >> > -Daniel
> >>
> >> Damien, can you comment? I could have sworn you said something different
> >> on IRC. It sounded like exactly what I wanted.
> >
> > What Daniel says is correct, the check is part of the runner wrapper,
> > not piglit itself.
> >
> > I'd rather have a environement check up-front and I don't mind where it
> > lives (igt Vs piglit).
> 
> The problem is that generating the testlist (or printing the commands)
> is a feature QA actually relies on. I also use it occasionally to
> quickly test igt library changes. So we can't bail that early. My
> patch bails fairly late, but I didn't see a better spot.
> -Daniel

I'm confused then about how this really improves my current situation.

-- 
Ben Widawsky, Intel Open Source Technology Center

[PATCH] tests/igt: Add runtime environment checks

[Daniel Vetter]

This is one of the nice pieces that I've never ported from the old
make based test runner. Note that we only use the result of the check
when actually running the testcases so that enumerating tests still
works as non-root on arbitrary machines.

v2: Fail the tests harder.

Cc: Ben Widawsky <ben-Egm8mFWDmf3k1uMJSBkQmQ@public.gmane.org>
Requested-by: Ben Widawsky <ben-Egm8mFWDmf3k1uMJSBkQmQ@public.gmane.org>
Signed-off-by: Daniel Vetter <daniel.vetter-/w4YWyX8dFk@public.gmane.org>
---
 tests/igt.tests | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/tests/igt.tests b/tests/igt.tests
index f3884925deaa..df747e3fac78 100644
--- a/tests/igt.tests
+++ b/tests/igt.tests
@@ -28,7 +28,7 @@ import sys
 import subprocess
 
 from os import path
-from framework.core import testBinDir, TestProfile
+from framework.core import testBinDir, TestProfile, TestResult
 from framework.exectest import ExecTest
 
 #############################################################################
@@ -39,6 +39,24 @@ from framework.exectest import ExecTest
 ##### automatically add all tests into the 'igt' category.
 #############################################################################
 
+def checkEnvironment():
+    debugfs_path = "/sys/kernel/debug/dri"
+    if os.getuid() != 0:
+        print "Test Environment check: not root!"
+        return False
+    if not os.path.isdir(debugfs_path):
+        print "Test Environment check: debugfs not mounted properly!"
+        return False
+    for subdir in os.listdir(debugfs_path):
+        clients = open(os.path.join(debugfs_path, subdir, "clients"), 'r')
+        lines = clients.readlines()
+        if len(lines) > 2:
+            print "Test Environment check: other drm clients running!"
+            return False
+
+    print "Test Environment check: Succeeded."
+    return True
+
 if not os.path.exists(os.path.join(testBinDir, 'igt')):
     print "igt symlink not found!"
     sys.exit(0)
@@ -46,6 +64,8 @@ if not os.path.exists(os.path.join(testBinDir, 'igt')):
 # Chase the piglit/bin/igt symlink to find where the tests really live.
 igtTestRoot = path.join(path.realpath(path.join(testBinDir, 'igt')), 'tests')
 
+igtEnvironmentOk = checkEnvironment()
+
 profile = TestProfile()
 
 class IGTTest(ExecTest):
@@ -54,6 +74,9 @@ class IGTTest(ExecTest):
         self.timeout = 60*20 # 20 minutes deadline by default
 
     def interpretResult(self, out, returncode, results, dmesg):
+        if not igtEnvironmentOk:
+            return out
+
         if returncode == 0:
             results['result'] = 'dmesg-warn' if dmesg != '' else 'pass'
         elif returncode == 77:
@@ -63,6 +86,13 @@ class IGTTest(ExecTest):
         return out
     def run(self, env):
         env.dmesg = True
+        if not igtEnvironmentOk:
+            results = TestResult()
+            results['result'] = 'fail'
+            results['info'] = unicode("Test Environment isn't OK")
+
+            return results
+
         return ExecTest.run(self, env)
 
 def listTests(listname):
-- 
1.8.1.4

Re: [PATCH] tests/igt: Add runtime environment checks

[Daniel Vetter]

On Tue, Nov 26, 2013 at 9:02 PM, Ben Widawsky <ben-Egm8mFWDmf3k1uMJSBkQmQ@public.gmane.org> wrote:
>> The problem is that generating the testlist (or printing the commands)
>> is a feature QA actually relies on. I also use it occasionally to
>> quickly test igt library changes. So we can't bail that early. My
>> patch bails fairly late, but I didn't see a better spot.
>> -Daniel
>
> I'm confused then about how this really improves my current situation.

Quick recap of our irc discussion: I guess I'm trying to solve a
slightly different problem, assuming that we always want some kind of
test result. Hence everything fails (as of v2) and the result json
contains the reason. I think that's the right thing to do for
regression testing in a lab-like setting.

For developers I guess a little script or so some option to yell
louder and faster than this here could still be useful. Otoh there'll
always be test-specific failure modes (like runtime pm not configured
properly) and imo it doesn't make much sense to filter for all of them
beforehand. And otherwise I kinda expect that people will cook their
own scripts anyway to kill X, Wayland and a bunch of deamons that get
in the way and then run piglit with a few default options.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch