On 09/12/2012 07:51 AM, Tom Stellard wrote: > On Thu, Sep 06, 2012 at 10:44:18PM +0200, Blaž Tomažič wrote: >> On čet, 2012-09-06 at 09:16 -0700, Eric Anholt wrote: >>> Blaž Tomažič <blaz.toma...@gmail.com> writes: >>> >>>> On tor, 2012-09-04 at 07:14 -0700, Tom Stellard wrote: >>>>> Some of the cl-util files contain some really long lines, especially >>>>> piglit-framework-cl-program.c:96-99. You should go through and wrap some >>>>> of the longer lines at 80 characters to make them easier to read. >>>> >>>> I went through all the code and wrapped as much of it as possible to 80 >>>> lines. >>>> >>>> The new fixed branch is now at: >>>> git://github.com/blazt/piglit.git opencl-request-v2 >>> >>> I haven't read through everything, but I've looked at the docs and >>> browsed some commits and I like it. I'd be happy to see it merged. >>> >>> A few recommendations I'd give for further developent: >>> >>> Add a bit of python too all_cl.tests to autodetect your .cl tests. It's >>> irritating when writing tests to remember to add it, and you end up >>> mistyping a filename and not running all_cl.tests and pushing the code >>> with a broken all_cl.tests. (This means losing nicely-formatted names >>> in the results, and replacing that with filenames. >> >> I can add an option to query for test's name since you can define it >> in .cl files. This way we don't lose nicely-formatted names. Or even >> better, use the option you provided next. >> >>> I've been thinking >>> it would be good to add support to the framework for part of the test >>> result to be the name to report the result under, which would open the >>> way for a single test binary that tests a few small things, or >>> variations, and reports them separately) >> >> Do you mean just adding something like this to the output of a test: >> PIGLIT: { 'name': 'Name', 'result': 'pass' } >> so the test binary can name itself (depending on each different run)? >> >> Or also that a test binary can report that line multiple times with >> different names? For example to report results of "subtests". >> >>> Drop the "plain" thing in all_cl.tests. It's an uninteresting artifact >>> of the class name that happens to do the work. >>> >>> Always use concurrent testing when possible. As you get more and more >>> tests, being able to light up all your cores for testing is really nice. >> >> This is meant for further development or to fix the current pull >> request? >> > > Hi Eric, > > Is the code ready to be pulled as is or does Blažneed to make these > changes first? > > -Tom
Eric's on vacation and then immediately heading to XDC, so I doubt he'll be able to respond any time soon. In my mind, these all seem like things that could get fixed in follow-on patches. _______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit