[Piglit] [Patch v2 1/4] log_tests.py: Add tests from framework/log.py
Dylan Baker
baker.dylan.c at gmail.com
Wed Feb 19 13:47:11 PST 2014
On Wednesday, February 19, 2014 04:39:40 PM Ilia Mirkin wrote:
> On Wed, Feb 19, 2014 at 4:24 PM, Dylan Baker <baker.dylan.c at gmail.com>
wrote:
> > On Wednesday, February 19, 2014 03:44:03 PM Ilia Mirkin wrote:
> >> On Wed, Feb 19, 2014 at 3:13 PM, Dylan Baker <baker.dylan.c at gmail.com>
> >
> > wrote:
> >> > On Wednesday, February 19, 2014 02:25:15 PM Ilia Mirkin wrote:
> >> >> On Wed, Feb 19, 2014 at 9:57 AM, Dylan Baker <baker.dylan.c at gmail.com>
> >> >
> >> > wrote:
> >> >> > On Tuesday, February 18, 2014 07:07:19 PM Ilia Mirkin wrote:
> >> >> >> On Tue, Feb 18, 2014 at 6:41 PM, Tom Stellard <tom at stellard.net>
> >
> > wrote:
> >> >> >> > Hi Dylan,
> >> >> >> >
> >> >> >> > I've tested version 2 of this series, and I have a few
> >> >> >> > questions/comments:
> >> >> >> >
> >> >> >> > + I really like being able to see how many tests have run and
> >> >> >> >
> >> >> >> > how many have completed. The pass/fail rates are nice and
> >> >> >> > help me identify bad test runs right away.
> >> >> >> >
> >> >> >> > + Would it be possible to print the test names in the non-verbose
> >> >> >> >
> >> >> >> > output mode? Would this work in concurrency mode?
> >> >> >> >
> >> >> >> > + When I use the verbose output, the output looks like this:
> >> >> >> >
> >> >> >> > running :: Program/Execute/get-global-id, skip: 11, warn: 1
> >> >> >> > Running
> >> >> >> > Test(s): 253
> >> >> >>
> >> >> >> Bah, the test name is too short... the OpenCL tests need to get
> >> >> >> with
> >> >> >> the program -- at least 50 chars per test name, like the OpenGL
> >> >> >> tests
> >> >> >
> >> >> > Yeah, that's why the short mode has the running test numbers. I
> >> >> > guess
> >> >> > we
> >> >> > could do something like have piglit catch SIGQUIT and print the
> >> >> > running
> >> >> > test names before exiting. Of course, that's the kind of evil thing
> >> >> > yum
> >> >> > does and drives me crazy
> >> >>
> >> >> Well, apparently that's the preferred way of killing piglit right now.
> >> >> I'm also not a big fan of doing unexpected things with signals.
> >> >>
> >> >> >> :) The old contents of the line aren't being fully overwritten...
> >> >> >>
> >> >> >> I wonder if instead of relying on \r + overwriting, we should
> >> >> >> instead
> >> >> >> emit a ^L (clear screen) -- although obviously not for the verbose
> >> >> >> output. I also wonder if this style of output should only happen if
> >> >> >> the output is a tty. This would also allow one to show all the
> >> >> >> currently-running tests, one per line or something (to satisfy the
> >> >> >> previous request). And it would resolve the issue when the line
> >> >> >> becomes longer than the terminal width and you get wrapping.
> >> >> >
> >> >> > we could also just emit blank space to pad up to 80 characters(since
> >> >> > that's
> >> >> > the standard width for a terminal), or we could do some magic to
> >> >> > find
> >> >> > the
> >> >> > width of the terminal, and do some more magic to fill space pad
> >> >> > that.
> >> >> >
> >> >> > If we really want to get fancy we could use curses to have
> >> >> > multi-line
> >> >> > rewrites. I looked into it initially and there are python bindings
> >> >> > that
> >> >> > work on all of the major versions of curses, but ewww... curses
> >> >>
> >> >> Well, we wouldn't have to go full-on curses, just a couple of commands
> >> >> that would be easy enough to just emit directly. Actually I guess ^L
> >> >> doesn't "just work" -- the shell must handle it somehow :( The
> >> >> internet says that print "\033[2J" "\033[0;0H" should work (clear
> >> >> screen, reset coordinates). But we can obviously only play such tricks
> >> >> at all in the first place if stdout is a tty, i.e.
> >> >> sys.stdout.isatty().
> >> >
> >> > I'm concerned about portability. We do have windows users, and then
> >> > there's
> >> > the bash, dash, tcsh, zsh, ksh, tmux, screen, etc. \r is pretty
> >> > standard.
> >>
> >> That sequence should work in any terminal written in the past 30
> >> years, AFAIK. However windows is an interesting question -- I would
> >> guess that isatty() returns false there. And then you would fall back
> >> on the terser output without test names which can still fit on one
> >> line. If isatty() is true there, we could just have an explicit check.
> >> A question is whether isatty() == false should imply verbose mode or
> >> not. (e.g. what should happen if you do ./piglit-run.py > foo.log, or
> >> even | tee foo.log)
> >
> > The other thing about the terminal codes is that they don't solve the
> > problem, since clearing the screen in verbose mode would defeat the
> > purpose of having a verbose mode (lots of terminal spew), and in terse
> > mode the line will only ever get longer.
>
> This would obviously not be done for verbose mode, only non-verbose.
> Otherwise, as you mention, it would completely defeat the point of
> verbose.
>
> But for the non-verbose mode, you could have the output like
>
> [1234/5000] pass: 1000, fail: 123, etc. Running tests:
> [1233] some/long/test-name with/many-parameters that/make-no-sense
> [1222] another/similarly-randomly-named/test
>
> And to refresh it, you'd clear the screen, and "redraw".
>
> > The other option is to have a -o/--out option that logs the output to a
> > file, and a -s/--silent option that silences sys.stdout (which could be
> > used in conjunction to get only file logging)
>
> I dunno if _more_ options are the answer :) I'd rather find the
> smallest number of options that maximizes people's satisfaction with
> the system. I was hoping that -v/not-v would be enough. If enough
> people would do -v by default, perhaps we should flip it and make it
> -q for quiet mode. (I've never seen -s for silent, -q is pretty
> standard, e.g. wget uses it, and a ton of other tools.) And using
> isatty() to seed it wouldn't be so bad -- tty's get quiet, non-tty's
> get verbose. Perhaps that's too confusing though.
That is an option, I'll code something up.
I acutally do have a use for a quite mode, I'm setting up a few headless
jenkins servers, and printing anything is utterly useless for that
application.
>
> >> >> >> And verbose mode could live without the summary.
> >> >> >
> >> >> > Is that a vote to drop summary from verbose?
> >> >>
> >> >> Even though I'm the one that suggested it -- yes. But bring the
> >> >> [xx/yy] back into the print then so that there's some indication of
> >> >> progress.
> >> >
> >> > So right now verbose is:
> >> > "{result} :: {name}\n"
> >> > "[{percent}] {summary} {running}\r"
> >> >
> >> > and terse is:
> >> > "[{percent}] {summary} {running}\r"
> >> >
> >> > I think that leaving {summary} is more valuable in verbose than
> >> > {running},
> >> > and that will solve the "mixed lines" problem in most cases (with dmesg
> >> > on, if you generate 100 or so of each status it still might be
> >> > problematic if the test name is short, but that seems like a bit of a
> >> > corner case). I'm also thinking that if on verbose we add a couple of
> >> > spaces after the last element that will help with any mixing that does
> >> > happen:
> >> > "{result} :: {name} \n"
> >> > "[{percent}] {summary}\r"
> >>
> >> But this was precisely Tom's situation -- the summary was too
> >> long/test name too short. Another other other thing we could do is
> >> make the summary shorter, e.g. 1-2 letters per status (P/F/DW/DF/S) or
> >> something like that.
> >
> > I was actually thinking about that too.
> >
> > Again, I'm left wanting python3, which has a function to get the width of
> > the terminal. With that it would be simple to just pad out the back with
> > spaces or force a maximum line length.
> >
> >> >> >> > It looks like the "running" lines are being merged with the
> >> >> >> > "status"
> >> >> >> > lines.
> >> >> >> > Is there any way to fix this, it makes the output hard to read.
> >> >> >> >
> >> >> >> > + When I use piglit I need to be able to determine the following
> >> >> >> > from
> >> >> >> >
> >> >> >> > the output:
> >> >> >> >
> >> >> >> > 1. Whether or not the test run is invalid. Meaning that I have
> >> >> >> > some
> >> >> >> >
> >> >> >> > configuration problem on my system which is causing all the
> >> >> >> > tests
> >> >> >> > to fail. With the old output I would do this by watching the
> >> >> >> > first
> >> >> >> > 20
> >> >> >> > and checking if they were all fails. With the new output I
> >> >> >> > can
> >> >> >> > do
> >> >> >> > this by looking at the pass/fail stats and/or enabling
> >> >> >> > verbose
> >> >> >> > output.
> >> >> >> >
> >> >> >> > 2. Which test is running when the system hangs. Enabling
> >> >> >> >
> >> >> >> > verbose output in the new logging allows me to do this.
> >> >> >>
> >> >> >> IME, a history is good for this too -- sometimes a test does
> >> >> >> something
> >> >> >> bad, but is able to exit before the system dies.
> >> >> >>
> >> >> >> > Thanks,
> >> >> >> > Tom
> >> >> >> >
> >> >> >> > On Tue, Feb 18, 2014 at 05:32:34AM -0800, Dylan Baker wrote:
> >> >> >> >> Signed-off-by: Dylan Baker <baker.dylan.c at gmail.com>
> >> >> >> >> Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>
> >> >> >> >> ---
> >> >> >> >>
> >> >> >> >> framework/tests/log_tests.py | 85
> >> >> >> >> ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 85
> >> >> >> >> insertions(+)
> >> >> >> >> create mode 100644 framework/tests/log_tests.py
> >> >> >> >>
> >> >> >> >> diff --git a/framework/tests/log_tests.py
> >> >> >> >> b/framework/tests/log_tests.py
> >> >> >> >> new file mode 100644
> >> >> >> >> index 0000000..5f0640f
> >> >> >> >> --- /dev/null
> >> >> >> >> +++ b/framework/tests/log_tests.py
> >> >> >> >> @@ -0,0 +1,85 @@
> >> >> >> >> +# Copyright (c) 2014 Intel Corporation
> >> >> >> >> +
> >> >> >> >> +# Permission is hereby granted, free of charge, to any person
> >> >> >> >> obtaining
> >> >> >> >> a copy +# of this software and associated documentation files
> >> >> >> >> (the
> >> >> >> >> "Software"), to deal +# in the Software without restriction,
> >> >> >> >> including
> >> >> >> >> without limitation the rights +# to use, copy, modify, merge,
> >> >> >> >> publish,
> >> >> >> >> distribute, sublicense, and/or sell +# copies of the Software,
> >> >> >> >> and
> >> >> >> >> to
> >> >> >> >> permit persons to whom the Software is +# furnished to do so,
> >> >> >> >> subject
> >> >> >> >> to
> >> >> >> >> the following conditions:
> >> >> >> >> +
> >> >> >> >> +# The above copyright notice and this permission notice shall
> >> >> >> >> be
> >> >> >> >> included in +# all copies or substantial portions of the
> >> >> >> >> Software.
> >> >> >> >> +
> >> >> >> >> +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
> >> >> >> >> KIND,
> >> >> >> >> EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE
> >> >> >> >> WARRANTIES
> >> >> >> >> OF
> >> >> >> >> MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND
> >> >> >> >> NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT
> >> >> >> >> HOLDERS
> >> >> >> >> BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER
> >> >> >> >> IN
> >> >> >> >> AN
> >> >> >> >> ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF
> >> >> >> >> OR
> >> >> >> >> IN
> >> >> >> >> CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> >> >> >> >> +#
> >> >> >> >> SOFTWARE.
> >> >> >> >> +
> >> >> >> >> +""" Module provides tests for log.py module """
> >> >> >> >> +
> >> >> >> >> +from types import * # This is a special * safe module
> >> >> >> >> +import nose.tools as nt
> >> >> >> >> +from framework.log import Log
> >> >> >> >> +
> >> >> >> >> +
> >> >> >> >> +def test_initialize_log():
> >> >> >> >> + """ Test that Log initializes with """
> >> >> >> >> + log = Log(100)
> >> >> >> >> + assert log
> >> >> >> >> +
> >> >> >> >> +
> >> >> >> >> +def test_get_current_return():
> >> >> >> >> + """ Test that pre_log returns a number """
> >> >> >> >> + log = Log(100)
> >> >> >> >> +
> >> >> >> >> + ret = log.get_current()
> >> >> >> >> + nt.assert_true(isinstance(ret, (IntType, FloatType,
> >> >> >> >> LongType)),
> >> >> >> >> + msg="Log.get_current() didn't return a
> >> >> >> >> numeric
> >> >> >> >> type!")
> >> >> >> >> +
> >> >> >> >> +
> >> >> >> >> +def test_mark_complete_increment_complete():
> >> >> >> >> + """ Tests that Log.mark_complete() increments
> >> >> >> >> self.__complete
> >> >> >> >> """
> >> >> >> >> + log = Log(100)
> >> >> >> >> + ret = log.get_current()
> >> >> >> >> + log.mark_complete(ret, 'pass')
> >> >> >> >> + nt.assert_equal(log._Log__complete, 1,
> >> >> >> >> + msg="Log.mark_complete() did not properly
> >> >> >> >> incremented " + "Log.__current")
> >> >> >> >> +
> >> >> >> >> +
> >> >> >> >> +def check_mark_complete_increment_summary(stat):
> >> >> >> >> + """ Test that passing a result to mark_complete works
> >> >> >> >> correctly
> >> >> >> >> """
> >> >> >> >> + log = Log(100)
> >> >> >> >> + ret = log.get_current()
> >> >> >> >> + log.mark_complete(ret, stat)
> >> >> >> >> + print log._Log__summary
> >> >> >> >> + nt.assert_equal(log._Log__summary[stat], 1,
> >> >> >> >> + msg="Log.__summary[{}] was not properly "
> >> >> >> >> + "incremented".format(stat))
> >> >> >> >> +
> >> >> >> >> +
> >> >> >> >> +def test_mark_complete_increment_summary():
> >> >> >> >> + """ Generator that creates tests for self.__summary """
> >> >> >> >> +
> >> >> >> >> +
> >> >> >> >> + valid_statuses = ('pass', 'fail', 'crash', 'warn',
> >> >> >> >> 'dmesg-warn',
> >> >> >> >> + 'dmesg-fail', 'skip')
> >> >> >> >> +
> >> >> >> >> + yieldable = check_mark_complete_increment_summary
> >> >> >> >> +
> >> >> >> >> + for stat in valid_statuses:
> >> >> >> >> + yieldable.description = ("Test that Log.mark_complete
> >> >> >> >> increments
> >> >> >> >> "
> >> >> >> >> +
> >> >> >> >> "self._summary[{}]".format(stat))
> >> >> >> >> + yield yieldable, stat
> >> >> >> >> +
> >> >> >> >> +
> >> >> >> >> +def test_mark_complete_removes_complete():
> >> >> >> >> + """ Test that Log.mark_complete() removes finished tests
> >> >> >> >> from
> >> >> >> >> __running """ + log = Log(100)
> >> >> >> >> + ret = log.get_current()
> >> >> >> >> + log.mark_complete(ret, 'pass')
> >> >> >> >> + nt.assert_not_in(ret, log._Log__running,
> >> >> >> >> + msg="Running tests not removed from
> >> >> >> >> running
> >> >> >> >> list")
> >> >> >> >> --
> >> >> >> >> 1.9.0
> >> >> >> >>
> >> >> >> >> _______________________________________________
> >> >> >> >> Piglit mailing list
> >> >> >> >> Piglit at lists.freedesktop.org
> >> >> >> >> http://lists.freedesktop.org/mailman/listinfo/piglit
> >> >> >> >
> >> >> >> > _______________________________________________
> >> >> >> > Piglit mailing list
> >> >> >> > Piglit at lists.freedesktop.org
> >> >> >> > http://lists.freedesktop.org/mailman/listinfo/piglit
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20140219/67a5df75/attachment-0001.pgp>
More information about the Piglit
mailing list