Re: [Piglit] [PATCH] shader_runner: Check return from program_must_be_in_use

[Brian Paul]

On 10/24/2016 11:32 AM, Dylan Baker wrote:

Which was changed in b300e1f58 to return a status, instead of exiting on
failure, but the calls to the function weren't updated to handle this
change.

This fixes the remaining uses of program_must_be_in_use not fixed by
Brian's patch.

cc: Brian Paul 
Signed-off-by: Dylan Baker 
---
  tests/shaders/shader_runner.c | 24 
  1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
index b0bde2c..3658210 100644
--- a/tests/shaders/shader_runner.c
+++ b/tests/shaders/shader_runner.c
@@ -3033,25 +3033,25 @@ piglit_display(void)
} else if (sscanf(line,
  "compute %d %d %d",
  , , ) == 3) {
-   program_must_be_in_use();
+   result = program_must_be_in_use();
glMemoryBarrier(GL_ALL_BARRIER_BITS);
glDispatchCompute(x, y, z);
glMemoryBarrier(GL_ALL_BARRIER_BITS);
} else if (sscanf(line,
  "compute group size %d %d %d %d %d %d",
  , , , , , ) == 6) {
-   program_must_be_in_use();
+   result = program_must_be_in_use();
glMemoryBarrier(GL_ALL_BARRIER_BITS);
glDispatchComputeGroupSizeARB(x, y, z, w, h, l);
glMemoryBarrier(GL_ALL_BARRIER_BITS);
} else if (string_match("draw rect tex", line)) {
-   program_must_be_in_use();
+   result = program_must_be_in_use();
program_subroutine_uniforms();
get_floats(line + 13, c, 8);
piglit_draw_rect_tex(c[0], c[1], c[2], c[3],
 c[4], c[5], c[6], c[7]);
} else if (string_match("draw rect ortho patch", line)) {
-   program_must_be_in_use();
+   result = program_must_be_in_use();
program_subroutine_uniforms();
get_floats(line + 21, c, 4);

@@ -3060,7 +3060,7 @@ piglit_display(void)
2.0 * (c[2] / piglit_width),
2.0 * (c[3] / piglit_height), 
true);
} else if (string_match("draw rect ortho", line)) {
-   program_must_be_in_use();
+   result = program_must_be_in_use();
program_subroutine_uniforms();
get_floats(line + 15, c, 4);

@@ -3069,18 +3069,18 @@ piglit_display(void)
 2.0 * (c[2] / piglit_width),
 2.0 * (c[3] / piglit_height));
} else if (string_match("draw rect patch", line)) {
-   program_must_be_in_use();
+   result = program_must_be_in_use();
get_floats(line + 15, c, 4);
piglit_draw_rect_custom(c[0], c[1], c[2], c[3], true);
} else if (string_match("draw rect", line)) {
-   program_must_be_in_use();
+   result = program_must_be_in_use();
program_subroutine_uniforms();
get_floats(line + 9, c, 4);
piglit_draw_rect(c[0], c[1], c[2], c[3]);
} else if (string_match("draw instanced rect", line)) {
int primcount;

-   program_must_be_in_use();
+   result = program_must_be_in_use();
sscanf(line + 19, "%d %f %f %f %f",
   ,
   c + 0, c + 1, c + 2, c + 3);
@@ -3089,7 +3089,7 @@ piglit_display(void)
GLenum mode = decode_drawing_mode(s);
int first = x;
size_t count = (size_t) y;
-   program_must_be_in_use();
+   result = program_must_be_in_use();
if (first < 0) {
printf("draw arrays 'first' must be >= 0\n");
piglit_report_result(PIGLIT_FAIL);
@@ -3483,10 +3483,10 @@ piglit_display(void)
} else if (string_match("texparameter ", line)) {
handle_texparameter(line + strlen("texparameter "));
} else if (string_match("uniform", line)) {
-   program_must_be_in_use();
+   result = program_must_be_in_use();
set_uniform(line + 7, ubo_array_index);
} else if (string_match("subuniform", 

[Piglit] [PATCH 24/27] framework/profile: Move group_manager from TestProfile to TestDict

[Dylan Baker]

This move is going to allow us to supplement the TestDict with a
different class that can be used instead that loads xml.

Signed-off-by: Dylan Baker 
---
 framework/profile.py| 104 ++
 tests/all.py| 508 ++---
 tests/cl.py |   8 +-
 tests/quick.py  |   8 +-
 tests/sanity.py |   2 +-
 unittests/framework/test_profile.py | 153 -
 6 files changed, 385 insertions(+), 398 deletions(-)

diff --git a/framework/profile.py b/framework/profile.py
index 29f71c1..30c1686 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -144,52 +144,6 @@ class TestDict(collections.MutableMapping):
 def __iter__(self):
 return iter(self.__container)
 
-@property
-@contextlib.contextmanager
-def allow_reassignment(self):
-"""Context manager that allows keys to be reassigned.
-
-Normally reassignment happens in error, but sometimes one actually
-wants to do reassignment, say to add extra options in a reduced
-profile. This method allows reassignment, but only within its context,
-making it an explicit choice to do so.
-
-It is safe to nest this contextmanager.
-
-This is not thread safe, or even co-routine safe.
-"""
-self.__allow_reassignment += 1
-yield
-self.__allow_reassignment -= 1
-
-
-class TestProfile(object):
-"""Class that holds a list of tests for execution.
-
-This class represents a single testsuite, it has a mapping (dictionary-like
-object) of tests attached (TestDict). This is a mapping of :
-(python 3 str, python 2 unicode), and the key is delimited by
-grouptools.SEPARATOR.
-
-The group_manager method provides a context_manager to make adding test to
-the test_list easier, by doing more validation and enforcement.
->>> t = TestProfile()
->>> with t.group_manager(Test, 'foo@bar') as g:
-... g(['foo'])
-
-This class does not provide a way to execute itself, instead that is
-handled by the run function in this module, which is able to process and
-run multiple TestProfile objects at once.
-"""
-def __init__(self):
-self.test_list = TestDict()
-self.forced_test_list = []
-self.filters = []
-self.options = {
-'dmesg': get_dmesg(False),
-'monitor': Monitoring(False),
-}
-
 @contextlib.contextmanager
 def group_manager(self, test_class, group, **default_args):
 """A context manager to make working with flat groups simple.
@@ -229,16 +183,15 @@ class TestProfile(object):
 """Helper function that actually adds the tests.
 
 Arguments:
-args -- arguments to be passed to the test_class constructor.
-This must be appropriate for the underlying class
+args   -- arguments to be passed to the test_class constructor.
+  This must be appropriate for the underlying class
 
 Keyword Arguments:
-name -- If this is a a truthy value that value will be used as the
-key for the test. If name is falsy then args will be
-' '.join'd and used as name. Default: None
+name   -- If this is a a truthy value that value will be used as
+  the key for the test. If name is falsy then args will be
+  ' '.join'd and used as name. Default: None
 kwargs -- Any additional args will be passed directly to the test
   constructor as keyword args.
-
 """
 # If there is no name, then either
 # a) join the arguments list together to make the name
@@ -255,7 +208,7 @@ class TestProfile(object):
 assert isinstance(name, six.string_types)
 lgroup = grouptools.join(group, name)
 
-self.test_list[lgroup] = test_class(
+self[lgroup] = test_class(
 args,
 **dict(itertools.chain(six.iteritems(default_args),
six.iteritems(kwargs
@@ -265,9 +218,48 @@ class TestProfile(object):
 @property
 @contextlib.contextmanager
 def allow_reassignment(self):
-"""A convenience wrapper around self.test_list.allow_reassignment."""
-with self.test_list.allow_reassignment:
-yield
+"""Context manager that allows keys to be reassigned.
+
+Normally reassignment happens in error, but sometimes one actually
+wants to do reassignment, say to add extra options in a reduced
+profile. This method allows reassignment, but only within its context,
+making it an explicit choice to do so.
+
+It is safe to nest this contextmanager.
+
+This is not thread safe, or even co-routine safe.
+"""

[Piglit] [PATCH 15/27] framework: Remove exclude_tests from options.OPTIONS

[Dylan Baker]

Rather than putting this in a global variable, just add a filter for
this in the runner. Far simpler, and removes more globals.

Signed-off-by: Dylan Baker 
---
 framework/options.py|  1 -
 framework/profile.py|  1 -
 framework/programs/run.py   |  5 -
 unittests/framework/test_profile.py | 17 -
 4 files changed, 4 insertions(+), 20 deletions(-)

diff --git a/framework/options.py b/framework/options.py
index 46e37ee..dc97c38 100644
--- a/framework/options.py
+++ b/framework/options.py
@@ -189,7 +189,6 @@ class _Options(object):  # pylint: 
disable=too-many-instance-attributes
 self.execute = True
 self._include_filter = _ReList()
 self._exclude_filter = _ReList()
-self.exclude_tests = set()
 self.valgrind = False
 self.dmesg = False
 self.monitored = False
diff --git a/framework/profile.py b/framework/profile.py
index 5404833..54e8e96 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -272,7 +272,6 @@ class TestProfile(object):
 """Filter for user-specified restrictions"""
 return ((not options.OPTIONS.include_filter or
  matches_any_regexp(path, options.OPTIONS.include_filter))
-and path not in options.OPTIONS.exclude_tests
 and not matches_any_regexp(path, 
options.OPTIONS.exclude_filter))
 
 filters = self.filters + [test_matches]
diff --git a/framework/programs/run.py b/framework/programs/run.py
index f07ee79..9d78d00 100644
--- a/framework/programs/run.py
+++ b/framework/programs/run.py
@@ -390,9 +390,10 @@ def resume(input_):
 
 # Don't re-run tests that have already completed, incomplete status tests
 # have obviously not completed.
+exclude_tests = set()
 for name, result in six.iteritems(results.tests):
 if args.no_retry or result.result != 'incomplete':
-options.OPTIONS.exclude_tests.add(name)
+exclude_tests.add(name)
 
 profiles = [profile.load_test_profile(p)
 for p in results.options['profile']]
@@ -405,6 +406,8 @@ def resume(input_):
 if options.OPTIONS.monitored:
 p.monitoring = options.OPTIONS.monitored
 
+p.filters.append(lambda n, _: n not in exclude_tests)
+
 # This is resumed, don't bother with time since it won't be accurate anyway
 profile.run(
 profiles,
diff --git a/unittests/framework/test_profile.py 
b/unittests/framework/test_profile.py
index 4ffabfa..f2aa5b5 100644
--- a/unittests/framework/test_profile.py
+++ b/unittests/framework/test_profile.py
@@ -154,23 +154,6 @@ class TestTestProfile(object):
 
 assert dict(profile_.test_list) == baseline
 
-def test_matches_env_exclude(self):
-"""profile.TestProfile.prepare_test_list: 'not path in
-env.exclude_tests'.
-"""
-# Pylint can't figure out that self.opts isn't a dict, but an
-# options._Option object.
-self.opts.exclude_tests.add(grouptools.join('group3', 'test5'))  # 
pylint: disable=no-member
-
-baseline = copy.deepcopy(self.data)
-del baseline[grouptools.join('group3', 'test5')]
-
-profile_ = profile.TestProfile()
-profile_.test_list = self.data
-profile_.prepare_test_list()
-
-assert dict(profile_.test_list) == dict(baseline)
-
 def test_matches_exclude_mar(self):
 """profile.TestProfile.prepare_test_list: 'not
 matches_any_regexp()'.
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH 27/27] tests/all.py: Make add_fbo_depthstencil_tests take an adder

[Dylan Baker]

Like all of the other helper functions, and will be required by later
patches in this series.

Signed-off-by: Dylan Baker 
---
 tests/all.py | 95 +
 1 file changed, 39 insertions(+), 56 deletions(-)

diff --git a/tests/all.py b/tests/all.py
index d07f436..ede3f17 100644
--- a/tests/all.py
+++ b/tests/all.py
@@ -63,16 +63,16 @@ def add_fbo_stencil_tests(adder, format):
   'fbo-stencil-{}-blit'.format(format))
 
 
-def add_fbo_depthstencil_tests(group, format, num_samples):
+def add_fbo_depthstencil_tests(adder, format, num_samples):
 assert format, \
 'add_fbo_depthstencil_tests argument "format" cannot be empty'
 
 if format == 'default_fb':
 prefix = ''
-create_test = lambda a: PiglitGLTest(a, run_concurrent=False)
+concurrent = False
 else:
 prefix = 'fbo-'
-create_test = PiglitGLTest
+concurrent = True
 
 if int(num_samples) > 1:
 suffix = ' samples=' + num_samples
@@ -81,51 +81,39 @@ def add_fbo_depthstencil_tests(group, format, num_samples):
 suffix = ''
 psamples = ''
 
-profile.test_list[grouptools.join(
-group, '{}depthstencil-{}-clear{}'.format(prefix, format, suffix))] = \
-create_test(['fbo-depthstencil', 'clear', format, psamples])
-profile.test_list[grouptools.join(
-group, '{}depthstencil-{}-readpixels-FLOAT-and-USHORT{}'.format(
-prefix, format, suffix))] = \
-create_test(['fbo-depthstencil', 'readpixels', format,
-'FLOAT-and-USHORT', psamples])
-profile.test_list[grouptools.join(
-group,
-'{}depthstencil-{}-readpixels-24_8{}'.format(
-prefix, format, suffix))] = \
-create_test(['fbo-depthstencil', 'readpixels', format, '24_8',
-psamples])
-profile.test_list[grouptools.join(
-group,
-'{}depthstencil-{}-readpixels-32F_24_8_REV{}'.format(
-prefix, format, suffix))] = \
-create_test(['fbo-depthstencil', 'readpixels', format,
-'32F_24_8_REV', psamples])
-profile.test_list[grouptools.join(
-group,
-'{}depthstencil-{}-drawpixels-FLOAT-and-USHORT{}'.format(
-prefix, format, suffix))] = \
-create_test(['fbo-depthstencil', 'drawpixels', format,
- 'FLOAT-and-USHORT', psamples])
-profile.test_list[grouptools.join(
-group,
-'{}depthstencil-{}-drawpixels-24_8{}'.format(
-prefix, format, suffix))] = \
-create_test(['fbo-depthstencil', 'drawpixels', format, '24_8',
-psamples])
-profile.test_list[grouptools.join(
-group,
-'{}depthstencil-{}-drawpixels-32F_24_8_REV{}'.format(
-prefix, format, suffix))] = \
-create_test(['fbo-depthstencil', 'drawpixels', format,
-'32F_24_8_REV', psamples])
-profile.test_list[grouptools.join(
-group,
-'{}depthstencil-{}-copypixels{}'.format(prefix, format, suffix))] = \
-create_test(['fbo-depthstencil', 'copypixels', format, psamples])
-profile.test_list[grouptools.join(
-group, '{}depthstencil-{}-blit{}'.format(prefix, format, suffix))] = \
-create_test(['fbo-depthstencil', 'blit', format, psamples])
+adder(['fbo-depthstencil', 'clear', format, psamples],
+  '{}depthstencil-{}-clear{}'.format(prefix, format, suffix),
+  run_concurrent=concurrent)
+adder(['fbo-depthstencil', 'readpixels', format, 'FLOAT-and-USHORT',
+   psamples],
+  '{}depthstencil-{}-readpixels-FLOAT-and-USHORT{}'.format(
+  prefix, format, suffix),
+  run_concurrent=concurrent)
+adder(['fbo-depthstencil', 'readpixels', format, '24_8', psamples],
+  '{}depthstencil-{}-readpixels-24_8{}'.format(prefix, format, suffix),
+  run_concurrent=concurrent)
+adder(['fbo-depthstencil', 'readpixels', format, '32F_24_8_REV', psamples],
+  '{}depthstencil-{}-readpixels-32F_24_8_REV{}'.format(
+  prefix, format, suffix),
+  run_concurrent=concurrent)
+adder(['fbo-depthstencil', 'drawpixels', format, 'FLOAT-and-USHORT',
+   psamples],
+  '{}depthstencil-{}-drawpixels-FLOAT-and-USHORT{}'.format(
+  prefix, format, suffix),
+  run_concurrent=concurrent)
+adder(['fbo-depthstencil', 'drawpixels', format, '24_8', psamples],
+  '{}depthstencil-{}-drawpixels-24_8{}'.format(prefix, format, suffix),
+  run_concurrent=concurrent)
+adder(['fbo-depthstencil', 'drawpixels', format, '32F_24_8_REV', psamples],
+  '{}depthstencil-{}-drawpixels-32F_24_8_REV{}'.format(
+  prefix, format, suffix),
+  run_concurrent=concurrent)
+adder(['fbo-depthstencil', 'copypixels', format, psamples],
+  '{}depthstencil-{}-copypixels{}'.format(prefix, 

[Piglit] [PATCH 10/27] framework/programs/run: Only allow --test-list if one profile

[Dylan Baker]

This limitation has basically always existed, but for the next patch to
function this is a requirement. This limitation will be properly fixed
later in this series.

Signed-off-by: Dylan Baker 
---
 framework/programs/run.py | 4 
 1 file changed, 4 insertions(+), 0 deletions(-)

diff --git a/framework/programs/run.py b/framework/programs/run.py
index 556878a..fefe7af 100644
--- a/framework/programs/run.py
+++ b/framework/programs/run.py
@@ -316,6 +316,10 @@ def run(input_):
 profile.results_dir = args.results_path
 # If a test list is provided then set the forced_test_list value.
 if args.test_list:
+if len(args.test_profiles) != 1:
+raise exceptions.PiglitFatalError(
+'Unable to force a test list with more than one profile')
+
 with open(args.test_list) as test_list:
 # Strip newlines
 profile.forced_test_list = list([t.strip() for t in test_list])
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH 7/27] framework/programs/run: Remove use of TestrunResult

[Dylan Baker]

We basically only used it to set the name and the time elapsed. That's
silly, just do those things directly. It needs less code and doesn't
require creating a big object.

This is leftover from the days before atomic writes, when results were
all stored in memory until the end of the run.

Signed-off-by: Dylan Baker 
---
 framework/programs/run.py | 19 ++-
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/framework/programs/run.py b/framework/programs/run.py
index 4c2227f..556878a 100644
--- a/framework/programs/run.py
+++ b/framework/programs/run.py
@@ -305,20 +305,12 @@ def run(input_):
 'Cannot overwrite existing folder without the -o/--overwrite '
 'option being set.')
 
-results = framework.results.TestrunResult()
-backends.set_meta(args.backend, results)
-
-# Set results.name
-if args.name is not None:
-results.name = args.name
-else:
-results.name = path.basename(args.results_path)
-
 backend = backends.get_backend(args.backend)(
 args.results_path,
 junit_suffix=args.junit_suffix,
 junit_subtests=args.junit_subtests)
-backend.initialize(_create_metadata(args, results.name))
+backend.initialize(_create_metadata(
+args, args.name or path.basename(args.results_path)))
 
 profile = framework.profile.merge_test_profiles(args.test_profile)
 profile.results_dir = args.results_path
@@ -328,7 +320,6 @@ def run(input_):
 # Strip newlines
 profile.forced_test_list = list([t.strip() for t in test_list])
 
-results.time_elapsed.start = time.time()
 # Set the dmesg type
 if args.dmesg:
 profile.dmesg = args.dmesg
@@ -336,10 +327,12 @@ def run(input_):
 if args.monitored:
 profile.monitoring = args.monitored
 
+time_elapsed = time.time()
+
 framework.profile.run(profile, args.log_level, backend, args.concurrent)
 
-results.time_elapsed.end = time.time()
-backend.finalize({'time_elapsed': results.time_elapsed})
+time_elapsed = time.time() - time_elapsed
+backend.finalize({'time_elapsed': time_elapsed})
 
 print('Thank you for running Piglit!\n'
   'Results have been written to ' + args.results_path)
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH 21/27] framework/profile: Split try/except block

[Dylan Baker]

This will avoid catching AttributeErrors when importing, and only catch
them if mod.profile doesn't exist.

Signed-off-by: Dylan Baker 
---
 framework/profile.py | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/framework/profile.py b/framework/profile.py
index cf8b2dd..6db65f1 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -356,15 +356,18 @@ def load_test_profile(filename):
 try:
 mod = importlib.import_module('tests.{0}'.format(
 os.path.splitext(os.path.basename(filename))[0]))
+except ImportError:
+raise exceptions.PiglitFatalError(
+'Failed to import "{}", there is either something wrong with the '
+'module or it doesn\'t exist. Check your spelling?'.format(
+filename))
+
+try:
 return mod.profile
 except AttributeError:
 raise exceptions.PiglitFatalError(
-'There is not profile attribute in module {}.\n'
+'There is no "profile" attribute in module {}.\n'
 'Did you specify the right file?'.format(filename))
-except ImportError:
-raise exceptions.PiglitFatalError(
-'There is no test profile called "{}".\n'
-'Check your spelling?'.format(filename))
 
 
 def run(profiles, logger, backend, concurrency):
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH 17/27] framework/profile: Factor out check_all closure

[Dylan Baker]

This does away with the check_all closure, by simplifying it down to an
equivalent lambda expression.

Signed-off-by: Dylan Baker 
---
 framework/profile.py | 14 +++---
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/framework/profile.py b/framework/profile.py
index cbbfa2d..e4f21d6 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -177,7 +177,7 @@ class TestDict(collections.MutableMapping):
 
 """
 for k, v in list(six.iteritems(self)):
-if not callable((k, v)):
+if not callable(k, v):
 del self[k]
 
 def reorder(self, order):
@@ -264,23 +264,15 @@ class TestProfile(object):
 runs it's own filters plus the filters in the self.filters name
 
 """
-def check_all(item):
-""" Checks group and test name against all filters """
-path, test = item
-for f in self.filters:
-if not f(path, test):
-return False
-return True
-
 if self.forced_test_list:
 # Remove all tests not in the test list, then reorder the tests to
 # match the testlist. This still allows additional filters to be
 # run afterwards.
-self.test_list.filter(lambda i: i[0] in self.forced_test_list)
+self.test_list.filter(lambda n, _: n in self.forced_test_list)
 self.test_list.reorder(self.forced_test_list)
 
 # Filter out unwanted tests
-self.test_list.filter(check_all)
+self.test_list.filter(lambda n, t: all(f(n, t) for f in self.filters))
 
 if not self.test_list:
 raise exceptions.PiglitFatalError(
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH 12/27] framework/profiles/run: remove redundant list() around comprehension

[Dylan Baker]

This is a list comprehension, it already results in a list. There is no
need to wrap it in the list() constructor call.

Signed-off-by: Dylan Baker 
---
 framework/programs/run.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/framework/programs/run.py b/framework/programs/run.py
index b30b292..be3bd3e 100644
--- a/framework/programs/run.py
+++ b/framework/programs/run.py
@@ -324,7 +324,7 @@ def run(input_):
 
 with open(args.test_list) as test_list:
 # Strip newlines
-profiles[0].forced_test_list = list([t.strip() for t in test_list])
+profiles[0].forced_test_list = [t.strip() for t in test_list]
 
 # Set the dmesg type
 if args.dmesg:
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH 25/27] framework/test/base: Remove timeout parameter

[Dylan Baker]

This doesn't work and never has, setting it would cause an exception,
and I'm not exactly sure how to fix it. It's also unused, so we can
bring it back later if we need it.

Signed-off-by: Dylan Baker 
---
 framework/test/base.py | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/framework/test/base.py b/framework/test/base.py
index 6b7cab6..4e7c8b2 100644
--- a/framework/test/base.py
+++ b/framework/test/base.py
@@ -175,7 +175,7 @@ class Test(object):
 __slots__ = ['run_concurrent', 'env', 'result', 'cwd', '_command']
 timeout = None
 
-def __init__(self, command, run_concurrent=False, timeout=None):
+def __init__(self, command, run_concurrent=False):
 assert isinstance(command, list), command
 
 self.run_concurrent = run_concurrent
@@ -183,9 +183,6 @@ class Test(object):
 self.env = {}
 self.result = TestResult()
 self.cwd = None
-if timeout is not None:
-assert isinstance(timeout, int)
-self.timeout = timeout
 
 def execute(self, path, log, options):
 """ Run a test
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH 11/27] framework/profile: Don't merge profiles

[Dylan Baker]

Because we can copy profiles, we don't need to merge them to run more
than one of them. Instead we can simply have a list of profiles, and run
them one by one. One side effect of this is that tests will be run one
profile at a time, so if running with out the -1/--no-concurrency or
-c/--all-concurrent options tests will run in a sort of zipper pattern:
, , , etc.

Signed-off-by: Dylan Baker 
---
 framework/profile.py| 93 +++---
 framework/programs/run.py   | 33 ++-
 unittests/framework/test_profile.py | 17 +-
 3 files changed, 55 insertions(+), 88 deletions(-)

diff --git a/framework/profile.py b/framework/profile.py
index 73a8a96..ed8166c 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -54,7 +54,6 @@ __all__ = [
 'ConcurrentMode',
 'TestProfile',
 'load_test_profile',
-'merge_test_profiles'
 ]
 
 
@@ -310,21 +309,6 @@ class TestProfile(object):
 """
 self.filters.append(function)
 
-def update(self, *profiles):
-""" Updates the contents of this TestProfile instance with another
-
-This method overwrites key:value pairs in self with those in the
-provided profiles argument. This allows multiple TestProfiles to be
-called in the same run; which could be used to run piglit and external
-suites at the same time.
-
-Arguments:
-profiles -- one or more TestProfile-like objects to be merged.
-
-"""
-for profile in profiles:
-self.test_list.update(profile.test_list)
-
 @contextlib.contextmanager
 def group_manager(self, test_class, group, prefix=None, **default_args):
 """A context manager to make working with flat groups simple.
@@ -453,24 +437,7 @@ def load_test_profile(filename):
 'Check your spelling?'.format(filename))
 
 
-def merge_test_profiles(profiles):
-""" Helper for loading and merging TestProfile instances
-
-Takes paths to test profiles as arguments and returns a single merged
-TestProfile instance.
-
-Arguments:
-profiles -- a list of one or more paths to profile files.
-
-"""
-profile = load_test_profile(profiles.pop())
-with profile.allow_reassignment:
-for p in profiles:
-profile.update(load_test_profile(p))
-return profile
-
-
-def run(profile, logger, backend, concurrency):
+def run(profiles, logger, backend, concurrency):
 """Runs all tests using Thread pool.
 
 When called this method will flatten out self.tests into self.test_list,
@@ -484,30 +451,50 @@ def run(profile, logger, backend, concurrency):
 Finally it will print a final summary of the tests.
 
 Arguments:
-profile -- a Profile ojbect.
-logger  -- a log.LogManager instance.
-backend -- a results.Backend derived instance.
+profiles -- a list of Profile instances.
+logger   -- a log.LogManager instance.
+backend  -- a results.Backend derived instance.
 """
 chunksize = 1
 
-profile.prepare_test_list()
-log = LogManager(logger, len(profile.test_list))
+for p in profiles:
+p.prepare_test_list()
+log = LogManager(logger, sum(len(p.test_list) for p in profiles))
 
-def test(pair, this_pool=None):
+def test(name, test, profile, this_pool=None):
 """Function to call test.execute from map"""
-name, test = pair
 with backend.write_test(name) as w:
 test.execute(name, log.get(), profile.dmesg, profile.monitoring)
 w(test.result)
 if profile.monitoring.abort_needed:
 this_pool.terminate()
 
-def run_threads(pool, testlist):
+def run_threads(pool, profile, filterby=None):
 """ Open a pool, close it, and join it """
-pool.imap(lambda pair: test(pair, pool), testlist, chunksize)
+iterable = six.iteritems(profile.test_list)
+if filterby:
+iterable = (x for x in iterable if filterby(x))
+
+pool.imap(lambda pair: test(pair[0], pair[1], profile, pool),
+  iterable, chunksize)
 pool.close()
 pool.join()
 
+def run_profile(profile):
+"""Run an individual profile."""
+if concurrency is ConcurrentMode.full:
+run_threads(multi, profile)
+elif concurrency is ConcurrentMode.none:
+run_threads(single, profile)
+else:
+assert concurrency is ConcurrentMode.some
+# Filter and return only thread safe tests to the threaded pool
+run_threads(multi, profile, lambda x: x[1].run_concurrent)
+
+# Filter and return the non thread safe tests to the single
+# pool
+run_threads(single, profile, lambda x: not x[1].run_concurrent)
+
 # Multiprocessing.dummy is a wrapper around Threading that provides a
 # multiprocessing compatible API
 #
@@ -516,21 +503,11 @@ def run(profile, logger, backend, 

[Piglit] [PATCH 23/27] framework/profile: remove unused argument from TestProfile.group_manager

[Dylan Baker]

Signed-off-by: Dylan Baker 
---
 framework/profile.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/framework/profile.py b/framework/profile.py
index 653dcec..29f71c1 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -191,7 +191,7 @@ class TestProfile(object):
 }
 
 @contextlib.contextmanager
-def group_manager(self, test_class, group, prefix=None, **default_args):
+def group_manager(self, test_class, group, **default_args):
 """A context manager to make working with flat groups simple.
 
 This provides a simple way to replace add_plain_test,
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH 26/27] framework/test/glsl_parser_test.py: split the parser out of the class

[Dylan Baker]

This pulls the code for parsing the test file out of GLSLParserTest and
puts them in a new Parser class. This is going be useful later for
allowing the parsing to be done during the compile stage, and not
require searching the file system at runtime.

Signed-off-by: Dylan Baker 
---
 framework/test/glsl_parser_test.py | 85 ---
 1 file changed, 46 insertions(+), 39 deletions(-)

diff --git a/framework/test/glsl_parser_test.py 
b/framework/test/glsl_parser_test.py
index b1acb18..22eb6ce 100644
--- a/framework/test/glsl_parser_test.py
+++ b/framework/test/glsl_parser_test.py
@@ -73,56 +73,45 @@ class 
GLSLParserInternalError(exceptions.PiglitInternalError):
 pass
 
 
-class GLSLParserTest(FastSkipMixin, PiglitBaseTest):
-""" Read the options in a glsl parser test and create a Test object
+class Parser(object):
+"""Find and parse the config block of a GLSLParserTest.
 
 Specifically it is necessary to parse a glsl_parser_test to get information
 about it before actually creating a PiglitTest. Even though this could
 be done with a function wrapper, making it a distinct class makes it easier
 to sort in the profile.
-
-Arguments:
-filepath -- the path to a glsl_parser_test which must end in .vert,
-.tesc, .tese, .geom or .frag
-
 """
 _CONFIG_KEYS = frozenset(['expect_result', 'glsl_version',
   'require_extensions', 'check_link'])
 
 def __init__(self, filepath):
-os.stat(filepath)
-
 # a set that stores a list of keys that have been found already
 self.__found_keys = set()
+self.gl_required = set()
+self.glsl_es_version = None
+self.glsl_version = None
 
-# Parse the config file and get the config section, then write this
-# section to a StringIO and pass that to ConfigParser
 try:
 with io.open(filepath, mode='r', encoding='utf-8') as testfile:
 testfile = testfile.read()
-config = self.__parser(testfile, filepath)
-command = self.__get_command(config, filepath)
+self.config = self.parse(testfile, filepath)
+self.command = self.get_command(filepath)
 except GLSLParserInternalError as e:
 raise exceptions.PiglitFatalError(
 'In file "{}":\n{}'.format(filepath, six.text_type(e)))
 
-super(GLSLParserTest, self).__init__(command, run_concurrent=True)
-
-self.__set_skip_conditions(config)
+self.set_skip_conditions()
 
-def __set_skip_conditions(self, config):
+def set_skip_conditions(self):
 """Set OpenGL and OpenGL ES fast skipping conditions."""
-glsl = config.get('glsl_version')
-if glsl:
-if _is_gles_version(glsl):
-self.glsl_es_version = float(glsl[:3])
-else:
-self.glsl_version = float(glsl)
+glsl = self.config['glsl_version']
+if _is_gles_version(glsl):
+self.glsl_es_version = float(glsl[:3])
+else:
+self.glsl_version = float(glsl)
 
-req = config.get('require_extensions')
-if req:
-self.gl_required = set(
-r for r in req.split() if not r.startswith('!'))
+req = self.config['require_extensions']
+self.gl_required = set(r for r in req.split() if not r.startswith('!'))
 
 # If GLES is requested, but piglit was not built with a gles version,
 # then ARB_ES3_compatibility is required. Add it to
@@ -138,10 +127,10 @@ class GLSLParserTest(FastSkipMixin, PiglitBaseTest):
 ver = '3_2'
 ext = 'ARB_ES{}_compatibility'.format(ver)
 self.gl_required.add(ext)
-self._command.append(ext)
+self.command.append(ext)
 
 @staticmethod
-def __pick_binary(version):
+def pick_binary(version):
 """Pick the correct version of glslparsertest to use.
 
 This will try to select glslparsertest_gles2 for OpenGL ES tests, and
@@ -162,7 +151,7 @@ class GLSLParserTest(FastSkipMixin, PiglitBaseTest):
 else:
 return 'None'
 
-def __get_command(self, config, filepath):
+def get_command(self, filepath):
 """ Create the command argument to pass to super()
 
 This private helper creates a configparser object, then reads in the
@@ -173,26 +162,26 @@ class GLSLParserTest(FastSkipMixin, PiglitBaseTest):
 
 """
 for opt in ['expect_result', 'glsl_version']:
-if not config.get(opt):
+if not self.config.get(opt):
 raise GLSLParserInternalError("Missing required section {} "
   "from config".format(opt))
 
 # Create the command and pass it into a PiglitTest()
-glsl = config['glsl_version']
+glsl = self.config['glsl_version']

[Piglit] [PATCH 14/27] framework/programs/run: import framework.profile as profile

[Dylan Baker]

Since there isn't a conflict in the name anymore, this just makes the
code cleaner and easier to read.

Signed-off-by: Dylan Baker 
---
 framework/programs/run.py | 23 +++
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/framework/programs/run.py b/framework/programs/run.py
index be3bd3e..f07ee79 100644
--- a/framework/programs/run.py
+++ b/framework/programs/run.py
@@ -33,8 +33,7 @@ import shutil
 import six
 
 from framework import core, backends, exceptions, options
-import framework.results
-import framework.profile
+from framework import profile
 from . import parsers
 
 __all__ = ['run',
@@ -126,14 +125,14 @@ def _run_parser(input_):
 conc_parser = parser.add_mutually_exclusive_group()
 conc_parser.add_argument('-c', '--all-concurrent',
  action="store_const",
- default=framework.profile.ConcurrentMode.some,
- const=framework.profile.ConcurrentMode.full,
+ default=profile.ConcurrentMode.some,
+ const=profile.ConcurrentMode.full,
  dest="concurrent",
  help="Run all tests concurrently")
 conc_parser.add_argument("-1", "--no-concurrency",
  action="store_const",
- default=framework.profile.ConcurrentMode.some,
- const=framework.profile.ConcurrentMode.none,
+ default=profile.ConcurrentMode.some,
+ const=profile.ConcurrentMode.none,
  dest="concurrent",
  help="Disable concurrent test runs")
 parser.add_argument("-p", "--platform",
@@ -274,7 +273,7 @@ def run(input_):
 # If dmesg is requested we must have serial run, this is because dmesg
 # isn't reliable with threaded run
 if args.dmesg or args.monitored:
-args.concurrent = framework.profile.ConcurrentMode.none
+args.concurrent = profile.ConcurrentMode.none
 
 # Pass arguments into Options
 options.OPTIONS.exclude_filter = args.exclude_tests
@@ -312,7 +311,7 @@ def run(input_):
 backend.initialize(_create_metadata(
 args, args.name or path.basename(args.results_path)))
 
-profiles = [framework.profile.load_test_profile(p) for p in 
args.test_profile]
+profiles = [profile.load_test_profile(p) for p in args.test_profile]
 for p in profiles:
 p.results_dir = args.results_path
 
@@ -337,7 +336,7 @@ def run(input_):
 
 time_elapsed = time.time()
 
-framework.profile.run(profiles, args.log_level, backend, args.concurrent)
+profile.run(profiles, args.log_level, backend, args.concurrent)
 
 time_elapsed = time.time() - time_elapsed
 backend.finalize({'time_elapsed': time_elapsed})
@@ -395,7 +394,7 @@ def resume(input_):
 if args.no_retry or result.result != 'incomplete':
 options.OPTIONS.exclude_tests.add(name)
 
-profiles = [framework.profile.load_test_profile(p)
+profiles = [profile.load_test_profile(p)
 for p in results.options['profile']]
 for p in profiles:
 p.results_dir = args.results_path
@@ -407,11 +406,11 @@ def resume(input_):
 p.monitoring = options.OPTIONS.monitored
 
 # This is resumed, don't bother with time since it won't be accurate anyway
-framework.profile.run(
+profile.run(
 profiles,
 results.options['log_level'],
 backend,
-framework.profile.ConcurrentMode[results.options['concurrent']])
+profile.ConcurrentMode[results.options['concurrent']])
 
 backend.finalize()
 
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH 1/27] framework/profile: drop pre and post run hooks.

[Dylan Baker]

These were never used for anything, and are just taking up space. They
also get in the way of changes that are coming later in this series.

Signed-off-by: Dylan Baker 
---
 framework/profile.py | 20 
 1 file changed, 0 insertions(+), 20 deletions(-)

diff --git a/framework/profile.py b/framework/profile.py
index 7b0cb07..1edab03 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -287,22 +287,6 @@ class TestProfile(object):
 raise exceptions.PiglitFatalError(
 'There are no tests scheduled to run. Aborting run.')
 
-def _pre_run_hook(self):
-""" Hook executed at the start of TestProfile.run
-
-To make use of this hook one will need to subclass TestProfile, and
-set this to do something, as be default it will no-op
-"""
-pass
-
-def _post_run_hook(self):
-""" Hook executed at the end of TestProfile.run
-
-To make use of this hook one will need to subclass TestProfile, and
-set this to do something, as be default it will no-op
-"""
-pass
-
 def run(self, logger, backend):
 """ Runs all tests using Thread pool
 
@@ -321,8 +305,6 @@ class TestProfile(object):
 
 """
 
-self._pre_run_hook()
-
 chunksize = 1
 
 self._prepare_test_list()
@@ -372,8 +354,6 @@ class TestProfile(object):
 log.get().summary()
 raise
 
-self._post_run_hook()
-
 if self._monitoring.abort_needed:
 raise exceptions.PiglitAbort(self._monitoring.error_message)
 
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH 3/27] piglit: Only catch parsed.func AttributeError

[Dylan Baker]

Fixes catching AttributeErrors it shouldn't.

Signed-off-by: Dylan Baker 
---
 piglit | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/piglit b/piglit
index 4ce6c41..6c1d30a 100755
--- a/piglit
+++ b/piglit
@@ -163,11 +163,11 @@ def main():
 # to that executable
 parsed, args = parser.parse_known_args(input_)
 try:
-returncode = parsed.func(args)
+runner = parsed.func
 except AttributeError:
 parser.print_help()
 sys.exit(1)
-sys.exit(returncode)
+sys.exit(runner(args))
 
 
 if __name__ == '__main__':
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH 9/27] tests: Copy profiles to allow them to be run in parallel

[Dylan Baker]

Some of these would be rather silly to run in parallel (xts and
xts-render, for example), but this will help avoid copy and pasting
things leading to the propagation of bad code.

Signed-off-by: Dylan Baker 
---
 tests/cpu.py| 4 +++-
 tests/glslparser.py | 4 +++-
 tests/gpu.py| 4 +++-
 tests/llvmpipe.py   | 4 +++-
 tests/quick.py  | 4 +++-
 tests/quick_cl.py   | 4 +++-
 tests/shader.py | 4 +++-
 tests/xts-render.py | 7 ---
 8 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/tests/cpu.py b/tests/cpu.py
index 34fb6f8..7fc905e 100644
--- a/tests/cpu.py
+++ b/tests/cpu.py
@@ -13,11 +13,13 @@ hardware.
 from __future__ import (
 absolute_import, division, print_function, unicode_literals
 )
-from tests.quick import profile
+from tests.quick import profile as _profile
 from framework.test import GLSLParserTest
 
 __all__ = ['profile']
 
+profile = _profile.copy()  # pylint: disable=invalid-name
+
 
 def filter_gpu(name, test):
 """Remove all tests that are run on the GPU."""
diff --git a/tests/glslparser.py b/tests/glslparser.py
index 60442a2..fccc353 100644
--- a/tests/glslparser.py
+++ b/tests/glslparser.py
@@ -5,8 +5,10 @@ from __future__ import (
 )
 
 from framework.test import GLSLParserTest
-from tests.all import profile
+from tests.all import profile as _profile
 
 __all__ = ['profile']
 
+profile = _profile.copy()  # pylint: disable=invalid-name
+
 profile.filter_tests(lambda _, t: isinstance(t, GLSLParserTest))
diff --git a/tests/gpu.py b/tests/gpu.py
index 01bca25..c9e3d15 100644
--- a/tests/gpu.py
+++ b/tests/gpu.py
@@ -6,11 +6,13 @@ from __future__ import (
 absolute_import, division, print_function, unicode_literals
 )
 
-from tests.quick import profile
+from tests.quick import profile as _profile
 from framework.test import GLSLParserTest
 
 __all__ = ['profile']
 
+profile = _profile.copy()  # pylint: disable=invalid-name
+
 # Remove all parser tests, as they are compiler test
 profile.filter_tests(lambda p, t: not isinstance(t, GLSLParserTest))
 profile.filter_tests(lambda n, _: not n.startswith('asmparsertest'))
diff --git a/tests/llvmpipe.py b/tests/llvmpipe.py
index f02755c..0ebd88b 100644
--- a/tests/llvmpipe.py
+++ b/tests/llvmpipe.py
@@ -8,10 +8,12 @@ import platform
 import sys
 
 from framework.grouptools import join
-from tests.gpu import profile
+from tests.gpu import profile as _profile
 
 __all__ = ['profile']
 
+profile = _profile.copy()  # pylint: disable=invalid-name
+
 
 def remove(key):
 try:
diff --git a/tests/quick.py b/tests/quick.py
index 0e02f92..7af9e82 100644
--- a/tests/quick.py
+++ b/tests/quick.py
@@ -6,13 +6,15 @@ from __future__ import (
 
 from framework import grouptools
 from framework.test import (GleanTest, PiglitGLTest)
-from tests.all import profile
+from tests.all import profile as _profile
 
 __all__ = ['profile']
 
 # See the note in all.py about this warning
 # pylint: disable=bad-continuation
 
+profile = _profile.copy()  # pylint: disable=invalid-name
+
 GleanTest.GLOBAL_PARAMS += ["--quick"]
 
 # Set the --quick flag on a few image_load_store_tests
diff --git a/tests/quick_cl.py b/tests/quick_cl.py
index 9f7c8f3..831e8fd 100644
--- a/tests/quick_cl.py
+++ b/tests/quick_cl.py
@@ -28,8 +28,10 @@ from __future__ import (
 absolute_import, division, print_function, unicode_literals
 )
 
-from tests.cl import profile
+from tests.cl import profile as _profile
 from framework.test import add_opencv_tests, add_oclconform_tests
 
+profile = _profile.copy()  # pylint: disable=invalid-name
+
 add_opencv_tests(profile)
 add_oclconform_tests(profile)
diff --git a/tests/shader.py b/tests/shader.py
index 3d67679..ed5635a 100644
--- a/tests/shader.py
+++ b/tests/shader.py
@@ -5,8 +5,10 @@ from __future__ import (
 )
 
 from framework.test.shader_test import ShaderTest, MultiShaderTest
-from tests.all import profile
+from tests.all import profile as _profile
 
 __all__ = ['profile']
 
+profile = _profile.copy()  # pylint: disable=invalid-name
+
 profile.filter_tests(lambda _, t: isinstance(t, (ShaderTest, MultiShaderTest)))
diff --git a/tests/xts-render.py b/tests/xts-render.py
index ee644be..234fb2f 100644
--- a/tests/xts-render.py
+++ b/tests/xts-render.py
@@ -22,11 +22,13 @@
 from __future__ import (
 absolute_import, division, print_function, unicode_literals
 )
-from framework import core
-from framework.profile import load_test_profile
+
+from tests.xts import profile as _profile
 
 __all__ = ['profile']
 
+profile = _profile.copy()  # pylint: disable=invalid-name
+
 
 def xts_render_filter(path, test):
 # Keep any tests that aren't from xts.
@@ -37,5 +39,4 @@ def xts_render_filter(path, test):
 return 'xlib9' in path
 
 
-profile = load_test_profile("xts")
 profile.filter_tests(xts_render_filter)
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org

[Piglit] [PATCH 4/27] framework: Split the run method out of profile.

[Dylan Baker]

There are a couple of reasons for doing this. First, profile is a big
complex mess that does entirely too much, and this helps with that.
Second, there are bugs in the way two profiles run at the same time
work, and this is going to fix that.

Signed-off-by: Dylan Baker 
---
 framework/profile.py| 131 ++---
 framework/programs/run.py   |   4 +-
 unittests/framework/test_profile.py |  10 +-
 3 files changed, 73 insertions(+), 72 deletions(-)

diff --git a/framework/profile.py b/framework/profile.py
index 53a17b7..b10f817 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -244,7 +244,7 @@ class TestProfile(object):
 """
 self._monitoring = Monitoring(monitored)
 
-def _prepare_test_list(self):
+def prepare_test_list(self):
 """ Prepare tests for running
 
 Flattens the nested group hierarchy into a flat dictionary using '/'
@@ -287,70 +287,6 @@ class TestProfile(object):
 raise exceptions.PiglitFatalError(
 'There are no tests scheduled to run. Aborting run.')
 
-def run(self, logger, backend):
-""" Runs all tests using Thread pool
-
-When called this method will flatten out self.tests into
-self.test_list, then will prepare a logger, and begin executing tests
-through it's Thread pools.
-
-Based on the value of options.OPTIONS.concurrent it will either run all
-the tests concurrently, all serially, or first the thread safe tests
-then the serial tests.
-
-Finally it will print a final summary of the tests
-
-Arguments:
-backend -- a results.Backend derived instance
-
-"""
-
-chunksize = 1
-
-self._prepare_test_list()
-log = LogManager(logger, len(self.test_list))
-
-def test(pair, this_pool=None):
-"""Function to call test.execute from map"""
-name, test = pair
-with backend.write_test(name) as w:
-test.execute(name, log.get(), self.dmesg, self.monitoring)
-w(test.result)
-if self._monitoring.abort_needed:
-this_pool.terminate()
-
-def run_threads(pool, testlist):
-""" Open a pool, close it, and join it """
-pool.imap(lambda pair: test(pair, pool), testlist, chunksize)
-pool.close()
-pool.join()
-
-# Multiprocessing.dummy is a wrapper around Threading that provides a
-# multiprocessing compatible API
-#
-# The default value of pool is the number of virtual processor cores
-single = multiprocessing.dummy.Pool(1)
-multi = multiprocessing.dummy.Pool()
-
-try:
-if options.OPTIONS.concurrent == "all":
-run_threads(multi, six.iteritems(self.test_list))
-elif options.OPTIONS.concurrent == "none":
-run_threads(single, six.iteritems(self.test_list))
-else:
-# Filter and return only thread safe tests to the threaded pool
-run_threads(multi, (x for x in six.iteritems(self.test_list)
-if x[1].run_concurrent))
-# Filter and return the non thread safe tests to the single
-# pool
-run_threads(single, (x for x in six.iteritems(self.test_list)
- if not x[1].run_concurrent))
-finally:
-log.get().summary()
-
-if self._monitoring.abort_needed:
-raise exceptions.PiglitAbort(self._monitoring.error_message)
-
 def filter_tests(self, function):
 """Filter out tests that return false from the supplied function
 
@@ -504,3 +440,68 @@ def merge_test_profiles(profiles):
 for p in profiles:
 profile.update(load_test_profile(p))
 return profile
+
+
+def run(profile, logger, backend):
+"""Runs all tests using Thread pool.
+
+When called this method will flatten out self.tests into self.test_list,
+then will prepare a logger, and begin executing tests through it's Thread
+pools.
+
+Based on the value of options.OPTIONS.concurrent it will either run all the
+tests concurrently, all serially, or first the thread safe tests then the
+serial tests.
+
+Finally it will print a final summary of the tests.
+
+Arguments:
+profile -- a Profile ojbect.
+logger  -- a log.LogManager instance.
+backend -- a results.Backend derived instance.
+"""
+chunksize = 1
+
+profile.prepare_test_list()
+log = LogManager(logger, len(profile.test_list))
+
+def test(pair, this_pool=None):
+"""Function to call test.execute from map"""
+name, test = pair
+with backend.write_test(name) as w:
+test.execute(name, log.get(), profile.dmesg, profile.monitoring)
+w(test.result)
+if 

[Piglit] [PATCH 20/27] framework/profile: Update docstrings

[Dylan Baker]

Signed-off-by: Dylan Baker 
---
 framework/profile.py | 101 
 1 file changed, 48 insertions(+), 53 deletions(-)

diff --git a/framework/profile.py b/framework/profile.py
index c118a67..cf8b2dd 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -19,11 +19,12 @@
 # OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
 # DEALINGS IN THE SOFTWARE.
 
-""" Provides Profiles for test groups
-
-Each set of tests, both native Piglit profiles and external suite integration,
-are represented by a TestProfile or a TestProfile derived object.
+"""Classes dealing with groups of Tests.
 
+In piglit tests are grouped into "profiles", which are equivalent to "suites"
+in some other testing nomenclature. A profile is a way to tell the framework
+that you have a group of tests you want to run, here are the names of those
+tests, and the Test instance.
 """
 
 from __future__ import (
@@ -69,18 +70,15 @@ class ConcurrentMode(enum.Enum):
 class TestDict(collections.MutableMapping):
 """A special kind of dict for tests.
 
-This dict lowers the names of keys by default.
-
-This class intentionally doesn't accept keyword arguments. This is
-intentional to avoid breakages.
+This mapping lowers the names of keys by default, and enforces that keys be
+strings (not bytes) and that values are Test derived objects. It is also a
+wrapper around collections.OrderedDict.
 
+This class doesn't accept keyword arguments, this is intentional. This is
+because the TestDict class is ordered, and keyword arguments are unordered,
+which is a design mismatch.
 """
 def __init__(self):
-# This is because it had special __setitem__ and __getitem__ protocol
-# methods, and simply passing *args and **kwargs into self.__container
-# will bypass these methods. It will also break the ordering, since a
-# regular dictionary or keyword arguments are inherintly unordered
-#
 # This counter is incremented once when the allow_reassignment context
 # manager is opened, and decremented each time it is closed. This
 # allows stacking of the context manager
@@ -90,15 +88,13 @@ class TestDict(collections.MutableMapping):
 def __setitem__(self, key, value):
 """Enforce types on set operations.
 
-Keys should only be strings, and values should only be more Trees
-or Tests.
+Keys should only be strings, and values should only be Tests.
 
 This method makes one additional requirement, it lowers the key before
 adding it. This solves a couple of problems, namely that we want to be
-able to use filesystem heirarchies as groups in some cases, and those
+able to use file-system hierarchies as groups in some cases, and those
 are assumed to be all lowercase to avoid problems on case insensitive
-filesystems.
-
+file-systems.
 """
 # keys should be strings
 if not isinstance(key, six.text_type):
@@ -156,12 +152,11 @@ class TestDict(collections.MutableMapping):
 Normally reassignment happens in error, but sometimes one actually
 wants to do reassignment, say to add extra options in a reduced
 profile. This method allows reassignment, but only within its context,
-making it an explict choice to do so.
+making it an explicit choice to do so.
 
 It is safe to nest this contextmanager.
 
-It is not safe to use this context manager in a threaded application
-
+This is not thread safe, or even co-routine safe.
 """
 self.__allow_reassignment += 1
 yield
@@ -169,22 +164,22 @@ class TestDict(collections.MutableMapping):
 
 
 class TestProfile(object):
-""" Class that holds a list of tests for execution
-
-This class provides a container for storing tests in either a nested
-dictionary structure (deprecated), or a flat dictionary structure with '/'
-delimited groups.
-
-Once a TestProfile object is created tests can be added to the test_list
-name as a key/value pair, the key should be a fully qualified name for the
-test, including it's group hierarchy and should be '/' delimited, with no
-leading or trailing '/', the value should be an exectest.Test derived
-object.
-
-When the test list is filled calling TestProfile.run() will set the
-execution of these tests off, and will flatten the nested group hierarchy
-of self.tests and merge it with self.test_list
-
+"""Class that holds a list of tests for execution.
+
+This class represents a single testsuite, it has a mapping (dictionary-like
+object) of tests attached (TestDict). This is a mapping of :
+(python 3 str, python 2 unicode), and the key is delimited by
+grouptools.SEPARATOR.
+
+The group_manager method provides a context_manager to make adding test to
+the 

[Piglit] [PATCH 5/27] framework: remove concurrent from OPTIONS, pass directly to profile

[Dylan Baker]

This removes a value out of the global OPTIONS, which is nice. It's very
trivial to pass this instead of putting it in options.

Signed-off-by: Dylan Baker 
---
 framework/options.py  |  2 --
 framework/profile.py  |  2 +-
 framework/programs/run.py | 11 +++
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/framework/options.py b/framework/options.py
index 5cb88aa..46e37ee 100644
--- a/framework/options.py
+++ b/framework/options.py
@@ -171,7 +171,6 @@ class _Options(object):  # pylint: 
disable=too-many-instance-attributes
 the configuration file.
 
 Options are as follows:
-concurrent -- one of: ["all", "some", "none"]. Default: "some"
 execute -- False for dry run
 include_filter -- list of compiled regex which include exclusively tests
   that match
@@ -187,7 +186,6 @@ class _Options(object):  # pylint: 
disable=too-many-instance-attributes
 exclude_filter = _ReListDescriptor('_exclude_filter', type_=_FilterReList)
 
 def __init__(self):
-self.concurrent = "some"
 self.execute = True
 self._include_filter = _ReList()
 self._exclude_filter = _ReList()
diff --git a/framework/profile.py b/framework/profile.py
index b10f817..821a630 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -442,7 +442,7 @@ def merge_test_profiles(profiles):
 return profile
 
 
-def run(profile, logger, backend):
+def run(profile, logger, backend, concurrency):
 """Runs all tests using Thread pool.
 
 When called this method will flatten out self.tests into self.test_list,
diff --git a/framework/programs/run.py b/framework/programs/run.py
index ba8437d..be4d75d 100644
--- a/framework/programs/run.py
+++ b/framework/programs/run.py
@@ -222,6 +222,7 @@ def _create_metadata(args, name):
 opts = dict(options.OPTIONS)
 opts['profile'] = args.test_profile
 opts['log_level'] = args.log_level
+opts['concurrent'] = args.concurrent
 if args.platform:
 opts['platform'] = args.platform
 
@@ -276,7 +277,6 @@ def run(input_):
 args.concurrency = "none"
 
 # Pass arguments into Options
-options.OPTIONS.concurrent = args.concurrency
 options.OPTIONS.exclude_filter = args.exclude_tests
 options.OPTIONS.include_filter = args.include_tests
 options.OPTIONS.execute = args.execute
@@ -336,7 +336,7 @@ def run(input_):
 if args.monitored:
 profile.monitoring = args.monitored
 
-framework.profile.run(profile, args.log_level, backend)
+framework.profile.run(profile, args.log_level, backend, args.concurrent)
 
 results.time_elapsed.end = time.time()
 backend.finalize({'time_elapsed': results.time_elapsed})
@@ -365,7 +365,6 @@ def resume(input_):
 _disable_windows_exception_messages()
 
 results = backends.load(args.results_path)
-options.OPTIONS.concurrent = results.options['concurrent']
 options.OPTIONS.exclude_filter = results.options['exclude_filter']
 options.OPTIONS.include_filter = results.options['include_filter']
 options.OPTIONS.execute = results.options['execute']
@@ -404,7 +403,11 @@ def resume(input_):
 profile.monitoring = options.OPTIONS.monitored
 
 # This is resumed, don't bother with time since it won't be accurate anyway
-framework.profile.run(profile, results.options['log_level'], backend)
+framework.profile.run(
+profile,
+results.options['log_level'],
+backend,
+results.options['concurrent'])
 
 backend.finalize()
 
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH 19/27] framework/profile: Update __all__

[Dylan Baker]

Signed-off-by: Dylan Baker 
---
 framework/profile.py | 2 ++
 1 file changed, 2 insertions(+), 0 deletions(-)

diff --git a/framework/profile.py b/framework/profile.py
index 837498e..c118a67 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -52,8 +52,10 @@ from framework.test.base import Test
 
 __all__ = [
 'ConcurrentMode',
+'TestDict',
 'TestProfile',
 'load_test_profile',
+'run',
 ]
 
 
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH 18/27] framework/profile: Don't alter the TestProfile before running

[Dylan Baker]

This patch changes the way that the tests from TestProfile are
processed. Rather than having a process_test_list method that modifies
TestProfile.test_list, a new itertests() method creates an iterator that
yields tests that are not excluded by filters.

This saves a bit of code, increases the memory usage, and reduces
surprise.

It would be nice if there wasn't a need to create a concrete test list,
but there wouldn't be any way to get the number of tests otherwise.

Signed-off-by: Dylan Baker 
---
 framework/profile.py| 104 ++---
 unittests/framework/test_profile.py |   6 +--
 2 files changed, 37 insertions(+), 73 deletions(-)

diff --git a/framework/profile.py b/framework/profile.py
index e4f21d6..837498e 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -165,35 +165,6 @@ class TestDict(collections.MutableMapping):
 yield
 self.__allow_reassignment -= 1
 
-def filter(self, callable):
-"""Filter tests out of the testdict before running.
-
-This method destructively filters results out of the test test
-dictionary list using the callable provided.
-
-Arguments:
-callable -- a callable object that returns truthy if the item remains,
-falsy if it should be removed
-
-"""
-for k, v in list(six.iteritems(self)):
-if not callable(k, v):
-del self[k]
-
-def reorder(self, order):
-"""Reorder the TestDict to match the order of the provided list."""
-new = collections.OrderedDict()
-try:
-for k in order:
-new[k] = self.__container[k]
-except KeyError:
-# If there is a name in order that isn't available in self there
-# will be a KeyError, this is expected. In this case fail
-# gracefully and report the error to the user.
-raise exceptions.PiglitFatalError(
-'Cannot reorder test: "{}", it is not in the 
profile.'.format(k))
-self.__container = new
-
 
 class TestProfile(object):
 """ Class that holds a list of tests for execution
@@ -256,28 +227,6 @@ class TestProfile(object):
 """
 self._monitoring = Monitoring(monitored)
 
-def prepare_test_list(self):
-""" Prepare tests for running
-
-Flattens the nested group hierarchy into a flat dictionary using '/'
-delimited groups by calling self.flatten_group_hierarchy(), then
-runs it's own filters plus the filters in the self.filters name
-
-"""
-if self.forced_test_list:
-# Remove all tests not in the test list, then reorder the tests to
-# match the testlist. This still allows additional filters to be
-# run afterwards.
-self.test_list.filter(lambda n, _: n in self.forced_test_list)
-self.test_list.reorder(self.forced_test_list)
-
-# Filter out unwanted tests
-self.test_list.filter(lambda n, t: all(f(n, t) for f in self.filters))
-
-if not self.test_list:
-raise exceptions.PiglitFatalError(
-'There are no tests scheduled to run. Aborting run.')
-
 @contextlib.contextmanager
 def group_manager(self, test_class, group, prefix=None, **default_args):
 """A context manager to make working with flat groups simple.
@@ -363,9 +312,8 @@ class TestProfile(object):
 
 This method creates a copy with references to the original instance
 (using copy.copy), except for the test_list attribute, which is copied
-using copy.deepcopy, which is necessary to ensure that
-prepare_test_list only affects the right instance. This allows profiles
-to be "subclassed" by other profiles, without modifying the original.
+using copy.deepcopy. This allows profiles to be "subclassed" by other
+profiles, without modifying the original.
 """
 new = copy.copy(self)
 new.test_list = copy.deepcopy(self.test_list)
@@ -373,6 +321,22 @@ class TestProfile(object):
 new.filters = copy.copy(self.filters)
 return new
 
+def itertests(self):
+"""Iterate over tests while filtering.
+
+This iterator is non-destructive.
+"""
+if self.forced_test_list:
+opts = collections.OrderedDict()
+for n in self.forced_test_list:
+opts[n] = self.test_list[n]
+else:
+opts = self.test_list  # pylint: disable=redefined-variable-type
+
+for k, v in six.iteritems(opts):
+if all(f(k, v) for f in self.filters):
+yield k, v
+
 
 def load_test_profile(filename):
 """Load a python module and return it's profile attribute.
@@ -426,9 +390,11 @@ def run(profiles, logger, backend, concurrency):
 """
 chunksize = 1
 
-for p in profiles:
-p.prepare_test_list()
-

[Piglit] [PATCH 8/27] framework/profile: add a copy method to profile

[Dylan Baker]

This will allow a profile to be copied and "subclassed" without
affecting the original profile. This will allow a long-standing bug that
made it impossible to run two subclasses of all.py (say shader.py and
glslparser.py) at the same time, since they would both try to modify the
all.py profile in incompatible ways.

Signed-off-by: Dylan Baker 
---
 framework/profile.py| 16 +-
 unittests/framework/test_profile.py | 52 ++-
 2 files changed, 68 insertions(+), 0 deletions(-)

diff --git a/framework/profile.py b/framework/profile.py
index 954bf25..73a8a96 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -31,6 +31,7 @@ from __future__ import (
 )
 import collections
 import contextlib
+import copy
 import importlib
 import itertools
 import multiprocessing
@@ -404,6 +405,21 @@ class TestProfile(object):
 with self.test_list.allow_reassignment:
 yield
 
+def copy(self):
+"""Create a copy of the TestProfile.
+
+This method creates a copy with references to the original instance
+(using copy.copy), except for the test_list attribute, which is copied
+using copy.deepcopy, which is necessary to ensure that filter_tests
+only affects the right instance. This allows profiles to be
+"subclassed" by other profiles, without modifying the original.
+"""
+new = copy.copy(self)
+new.test_list = copy.deepcopy(self.test_list)
+new.forced_test_list = copy.copy(self.forced_test_list)
+new.filters = copy.copy(self.filters)
+return new
+
 
 def load_test_profile(filename):
 """Load a python module and return it's profile attribute.
diff --git a/unittests/framework/test_profile.py 
b/unittests/framework/test_profile.py
index 6671349..5ef95e4 100644
--- a/unittests/framework/test_profile.py
+++ b/unittests/framework/test_profile.py
@@ -285,6 +285,58 @@ class TestTestProfile(object):
 
 assert grouptools.join('foo', 'abc') in self.profile.test_list
 
+class TestCopy(object):
+"""Tests for the copy method."""
+
+@pytest.fixture
+def fixture(self):
+orig = profile.TestProfile()
+orig.test_list['foo'] = utils.Test(['foo'])
+orig.test_list['bar'] = utils.Test(['bar'])
+orig.filters = [lambda name, _: name != 'bar']
+orig.forced_test_list = ['foo']
+return orig
+
+def test_filters(self, fixture):
+"""The filters attribute is copied correctly."""
+new = fixture.copy()
+
+# Assert that the fixtures are equivalent, but not the same
+assert fixture.filters == new.filters
+assert fixture.filters is not new.filters
+
+# And double check by modifying one of them and asserting that the
+# other has not changed.
+new.filters.append(lambda name, _: name != 'oink')
+assert len(fixture.filters) == 1
+
+def test_forced_test_list(self, fixture):
+"""The forced_test_list attribute is copied correctly."""
+new = fixture.copy()
+
+# Assert that the fixtures are equivalent, but not the same
+assert fixture.forced_test_list == new.forced_test_list
+assert fixture.forced_test_list is not new.forced_test_list
+
+# And double check by modifying one of them and asserting that the
+# other has not changed.
+del new.forced_test_list[0]
+assert fixture.forced_test_list[0] == 'foo'
+
+def test_test_list(self, fixture):
+"""The test_list attribute is copied correctly."""
+new = fixture.copy()
+
+# Assert that the fixtures are equivalent, but not the same
+assert fixture.test_list == new.test_list
+assert fixture.test_list is not new.test_list
+
+def test_prepare_test_list(self, fixture):
+"""The prepare_test_list method doesn't affect both."""
+new = fixture.copy()
+new.prepare_test_list()
+assert new.test_list != fixture.test_list
+
 
 class TestTestDict(object):
 """Tests for the TestDict object."""
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH] shader_runner: Check return from program_must_be_in_use

[Dylan Baker]

Which was changed in b300e1f58 to return a status, instead of exiting on
failure, but the calls to the function weren't updated to handle this
change.

This fixes the remaining uses of program_must_be_in_use not fixed by
Brian's patch.

cc: Brian Paul 
Signed-off-by: Dylan Baker 
---
 tests/shaders/shader_runner.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
index b0bde2c..3658210 100644
--- a/tests/shaders/shader_runner.c
+++ b/tests/shaders/shader_runner.c
@@ -3033,25 +3033,25 @@ piglit_display(void)
} else if (sscanf(line,
  "compute %d %d %d",
  , , ) == 3) {
-   program_must_be_in_use();
+   result = program_must_be_in_use();
glMemoryBarrier(GL_ALL_BARRIER_BITS);
glDispatchCompute(x, y, z);
glMemoryBarrier(GL_ALL_BARRIER_BITS);
} else if (sscanf(line,
  "compute group size %d %d %d %d %d %d",
  , , , , , ) == 6) {
-   program_must_be_in_use();
+   result = program_must_be_in_use();
glMemoryBarrier(GL_ALL_BARRIER_BITS);
glDispatchComputeGroupSizeARB(x, y, z, w, h, l);
glMemoryBarrier(GL_ALL_BARRIER_BITS);
} else if (string_match("draw rect tex", line)) {
-   program_must_be_in_use();
+   result = program_must_be_in_use();
program_subroutine_uniforms();
get_floats(line + 13, c, 8);
piglit_draw_rect_tex(c[0], c[1], c[2], c[3],
 c[4], c[5], c[6], c[7]);
} else if (string_match("draw rect ortho patch", line)) {
-   program_must_be_in_use();
+   result = program_must_be_in_use();
program_subroutine_uniforms();
get_floats(line + 21, c, 4);
 
@@ -3060,7 +3060,7 @@ piglit_display(void)
2.0 * (c[2] / piglit_width),
2.0 * (c[3] / piglit_height), 
true);
} else if (string_match("draw rect ortho", line)) {
-   program_must_be_in_use();
+   result = program_must_be_in_use();
program_subroutine_uniforms();
get_floats(line + 15, c, 4);
 
@@ -3069,18 +3069,18 @@ piglit_display(void)
 2.0 * (c[2] / piglit_width),
 2.0 * (c[3] / piglit_height));
} else if (string_match("draw rect patch", line)) {
-   program_must_be_in_use();
+   result = program_must_be_in_use();
get_floats(line + 15, c, 4);
piglit_draw_rect_custom(c[0], c[1], c[2], c[3], true);
} else if (string_match("draw rect", line)) {
-   program_must_be_in_use();
+   result = program_must_be_in_use();
program_subroutine_uniforms();
get_floats(line + 9, c, 4);
piglit_draw_rect(c[0], c[1], c[2], c[3]);
} else if (string_match("draw instanced rect", line)) {
int primcount;
 
-   program_must_be_in_use();
+   result = program_must_be_in_use();
sscanf(line + 19, "%d %f %f %f %f",
   ,
   c + 0, c + 1, c + 2, c + 3);
@@ -3089,7 +3089,7 @@ piglit_display(void)
GLenum mode = decode_drawing_mode(s);
int first = x;
size_t count = (size_t) y;
-   program_must_be_in_use();
+   result = program_must_be_in_use();
if (first < 0) {
printf("draw arrays 'first' must be >= 0\n");
piglit_report_result(PIGLIT_FAIL);
@@ -3483,10 +3483,10 @@ piglit_display(void)
} else if (string_match("texparameter ", line)) {
handle_texparameter(line + strlen("texparameter "));
} else if (string_match("uniform", line)) {
-   program_must_be_in_use();
+   result = program_must_be_in_use();
set_uniform(line + 7, ubo_array_index);
} else if (string_match("subuniform", line)) {
-   

Re: [Piglit] [PATCH 1/2] framework: change 'regressions' to mean 'regressions in the last test run'

[Dylan Baker]

I agree with Ilia that making regressions and fixes non-symmetric is bad. So at
the very least I'd want to see the same change to fixes.

The regressions page was designed that someone could run piglit using "git
rebase -x" and see what was regressing at each step along the way, and this
change would definitely break that workflow. Wether anyone does that (or
something like it) I don't know.

Honestly it's been on my todo for quite a while to either replace the HTML
summary with a program that is more flexible, or rewrite the HTML summary with
some javascript to have columns of selectable statuses so that everyone picks
what they want and just get it.

Dylan


signature.asc
Description: signature
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

Re: [Piglit] [PATCH 1/2] framework: change 'regressions' to mean 'regressions in the last test run'

[Ilia Mirkin]

On Mon, Oct 24, 2016 at 11:04 AM, Nicolai Hähnle  wrote:
> On 24.10.2016 16:57, Ilia Mirkin wrote:
>>
>> So is the idea that now regressions shows regressions compared to an
>> initial "golden" run?
>
>
> No, it's the dual of that: the latest run is compared to everything before,
> i.e. it answers the question: what's failing _now_ that used to work at some
> point in the past?
>
>
>> I don't like the idea of only changing regressions. These things were
>> previously pretty consistent...
>>
>> fixes + regressions + new + disabled = changes
>
>
> But this "sums" isn't disjoint: a test can appear in all of fixes,
> regressions, and new (and in some unusual cases even disabled)
> simultaneously. That makes the status quo pretty useless IMO.
>
> Maybe 'fixes' should get a similar do-over to 'regressions', but I'm not
> sure what that would look like.
>
> Let me turn this around and ask: (a) are you using the regressions page
> today, and (b) if so, how? Also, the same questions for the fixes page,
> because I have no use for that at all :)

I have two disjoint use-cases, neither of which will break as a result
of your changes:

(a) Put up a bunch of unrelated runs. In that case, only the problems
page really matters.

(b) Put up two runs, compare a new thing to an old thing. In that
case, I will generally look at the changes page, unless that thing is
massively polluted by a silly rename, in which case I'll look at fixes
+ regressions and disregard the new/disabled. With a scan over the
problems page in case anything obvious pops up on there.

However there's something to be said for logical consistency. Each
"diff" page working in the same way makes sense. Why is regressions
different from fixes?

  -ilia
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

Re: [Piglit] [PATCH 1/2] framework: change 'regressions' to mean 'regressions in the last test run'

[Nicolai Hähnle]

On 24.10.2016 16:57, Ilia Mirkin wrote:

So is the idea that now regressions shows regressions compared to an
initial "golden" run?


No, it's the dual of that: the latest run is compared to everything 
before, i.e. it answers the question: what's failing _now_ that used to 
work at some point in the past?




I don't like the idea of only changing regressions. These things were
previously pretty consistent...

fixes + regressions + new + disabled = changes


But this "sums" isn't disjoint: a test can appear in all of fixes, 
regressions, and new (and in some unusual cases even disabled) 
simultaneously. That makes the status quo pretty useless IMO.


Maybe 'fixes' should get a similar do-over to 'regressions', but I'm not 
sure what that would look like.


Let me turn this around and ask: (a) are you using the regressions page 
today, and (b) if so, how? Also, the same questions for the fixes page, 
because I have no use for that at all :)


Nicolai



Now regressions is some weird other thing not part of that taxonomy.
I'd rather that invariant remain - if you change one, you change all.

On Mon, Oct 24, 2016 at 10:02 AM, Nicolai Hähnle  wrote:

From: Nicolai Hähnle 

The idea is that 'regressions' can be useful when a summary is generated
for a sequence of chronologically sorted runs of the same configuration.
In that case, what's interesting is the state of the driver in the latest
run. With this change, 'regressions' now shows those tests that fail (or
crash etc.) in the latest run but have had a better result in at least one
earlier run.

In particular, this no longer shows tests that regressed in an earlier
test but have been fixed again since then.
---
 framework/summary/common.py | 29 +++--
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/framework/summary/common.py b/framework/summary/common.py
index 8c7e2c7..d3a7b31 100644
--- a/framework/summary/common.py
+++ b/framework/summary/common.py
@@ -23,20 +23,21 @@
 """Shared functions for summary generation."""

 from __future__ import (
 absolute_import, division, print_function, unicode_literals
 )
 import re
 import operator

 import six
 from six.moves import zip
+from itertools import repeat

 # a local variable status exists, prevent accidental overloading by renaming
 # the module
 import framework.status as so
 from framework.core import lazy_property
 from framework import grouptools


 class Results(object):  # pylint: disable=too-few-public-methods
 """Container object for results.
@@ -69,28 +70,29 @@ class Names(object):
 """Class containing names of tests for various statuses.

 Members contain lists of sets of names that have a status.

 Each status is lazily evaluated and cached.

 """
 def __init__(self, tests):
 self.__results = tests.results

-def __diff(self, comparator, handler=None):
+def __diff(self, comparator, handler=None, lhs=None, rhs=None):
 """Helper for simplifying comparators using find_diffs."""
 ret = ['']
 if handler is None:
-ret.extend(find_diffs(self.__results, self.all, comparator))
+ret.extend(find_diffs(self.__results, self.all, comparator,
+  lhs=lhs, rhs=rhs))
 else:
 ret.extend(find_diffs(self.__results, self.all, comparator,
-  handler=handler))
+  handler=handler, lhs=lhs, rhs=rhs))
 return ret

 def __single(self, comparator):
 """Helper for simplifying comparators using find_single."""
 return find_single(self.__results, self.all, comparator)

 @lazy_property
 def all(self):
 """A set of all tests in all runs."""
 all_ = set()
@@ -133,21 +135,22 @@ class Names(object):
 @lazy_property
 def skips(self):
 # It is critical to use is not == here, otherwise so.NOTRUN will also
 # be added to this list
 return self.__single(lambda x: x is so.SKIP)

 @lazy_property
 def regressions(self):
 # By ensureing tha min(x, y) is >= so.PASS we eleminate NOTRUN and SKIP
 # from these pages
-return self.__diff(lambda x, y: x < y and min(x, y) >= so.PASS)
+return self.__diff(lambda x, y: x < y and min(x, y) >= so.PASS,
+   rhs=self.__results[-1])

 @lazy_property
 def fixes(self):
 # By ensureing tha min(x, y) is >= so.PASS we eleminate NOTRUN and SKIP
 # from these pages
 return self.__diff(lambda x, y: x > y and min(x, y) >= so.PASS)

 @lazy_property
 def enabled(self):
 def handler(names, name, prev, cur):
@@ -285,41 +288,55 @@ def _result_in(name, result):
 """If a result (or a subtest result) exists return True, else False."""
 try:
 # This is a little hacky, but I don't know of a better way where we
 # ensure the 

[Piglit] [RFC] [PATCH 0/2] Changing the meaning of 'regressions'

[Nicolai Hähnle]

Hi all,

I found the 'regressions' page of the HTML summary to be pretty useless
if you're comparing more than two runs, since it ends up being very
similar to 'changes'.

The first patch makes 'regressions' more useful for me when comparing many
chronologically sorted runs. The second patch just extends it to the
console summary.

Obviously if there's anybody who is invested in the 'regressions' page
working as it is today we'll need to figure something out, but IMHO this
change is strictly an improvement in terms of usability.

Please review!
Thanks,
Nicolai


___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH 1/2] framework: change 'regressions' to mean 'regressions in the last test run'

[Nicolai Hähnle]

From: Nicolai Hähnle 

The idea is that 'regressions' can be useful when a summary is generated
for a sequence of chronologically sorted runs of the same configuration.
In that case, what's interesting is the state of the driver in the latest
run. With this change, 'regressions' now shows those tests that fail (or
crash etc.) in the latest run but have had a better result in at least one
earlier run.

In particular, this no longer shows tests that regressed in an earlier
test but have been fixed again since then.
---
 framework/summary/common.py | 29 +++--
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/framework/summary/common.py b/framework/summary/common.py
index 8c7e2c7..d3a7b31 100644
--- a/framework/summary/common.py
+++ b/framework/summary/common.py
@@ -23,20 +23,21 @@
 """Shared functions for summary generation."""
 
 from __future__ import (
 absolute_import, division, print_function, unicode_literals
 )
 import re
 import operator
 
 import six
 from six.moves import zip
+from itertools import repeat
 
 # a local variable status exists, prevent accidental overloading by renaming
 # the module
 import framework.status as so
 from framework.core import lazy_property
 from framework import grouptools
 
 
 class Results(object):  # pylint: disable=too-few-public-methods
 """Container object for results.
@@ -69,28 +70,29 @@ class Names(object):
 """Class containing names of tests for various statuses.
 
 Members contain lists of sets of names that have a status.
 
 Each status is lazily evaluated and cached.
 
 """
 def __init__(self, tests):
 self.__results = tests.results
 
-def __diff(self, comparator, handler=None):
+def __diff(self, comparator, handler=None, lhs=None, rhs=None):
 """Helper for simplifying comparators using find_diffs."""
 ret = ['']
 if handler is None:
-ret.extend(find_diffs(self.__results, self.all, comparator))
+ret.extend(find_diffs(self.__results, self.all, comparator,
+  lhs=lhs, rhs=rhs))
 else:
 ret.extend(find_diffs(self.__results, self.all, comparator,
-  handler=handler))
+  handler=handler, lhs=lhs, rhs=rhs))
 return ret
 
 def __single(self, comparator):
 """Helper for simplifying comparators using find_single."""
 return find_single(self.__results, self.all, comparator)
 
 @lazy_property
 def all(self):
 """A set of all tests in all runs."""
 all_ = set()
@@ -133,21 +135,22 @@ class Names(object):
 @lazy_property
 def skips(self):
 # It is critical to use is not == here, otherwise so.NOTRUN will also
 # be added to this list
 return self.__single(lambda x: x is so.SKIP)
 
 @lazy_property
 def regressions(self):
 # By ensureing tha min(x, y) is >= so.PASS we eleminate NOTRUN and SKIP
 # from these pages
-return self.__diff(lambda x, y: x < y and min(x, y) >= so.PASS)
+return self.__diff(lambda x, y: x < y and min(x, y) >= so.PASS,
+   rhs=self.__results[-1])
 
 @lazy_property
 def fixes(self):
 # By ensureing tha min(x, y) is >= so.PASS we eleminate NOTRUN and SKIP
 # from these pages
 return self.__diff(lambda x, y: x > y and min(x, y) >= so.PASS)
 
 @lazy_property
 def enabled(self):
 def handler(names, name, prev, cur):
@@ -285,41 +288,55 @@ def _result_in(name, result):
 """If a result (or a subtest result) exists return True, else False."""
 try:
 # This is a little hacky, but I don't know of a better way where we
 # ensure the value is truthy
 _ = result.get_result(name)
 return True
 except KeyError:
 return False
 
 
-def find_diffs(results, tests, comparator, handler=lambda *a: None):
+def find_diffs(results, tests, comparator, handler=lambda *a: None, lhs=None, 
rhs=None):
 """Generate diffs between two or more sets of results.
 
 Arguments:
 results -- a list of results.TestrunResult instances
 tests -- an iterable of test names. Must be iterable more than once
 comparator -- a function with the signautre f(x, y), that returns True when
   the test should be added to the set of diffs
+lhs -- the left-hand-side result for calls to comparator.
+   If not specified, results from the range results[:-1] will be used.
+rhs -- the right-hand-side result for calls to comparator.
+   If not specified, results from the range results[1:] will be used.
+   Note that at least one of lhs and rhs must be unspecified.
 
 Keyword Arguemnts:
 handler -- a function with the signature f(names, name, prev, cur). in the
event of a KeyError while comparing the results with 

[Piglit] [PATCH 2/2] framework: add -r/--regressions option to `piglit summary console`

[Nicolai Hähnle]

From: Nicolai Hähnle 

---
 framework/programs/summary.py | 5 +
 framework/summary/console_.py | 4 +++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/framework/programs/summary.py b/framework/programs/summary.py
index e400d9a..037bb56 100644
--- a/framework/programs/summary.py
+++ b/framework/programs/summary.py
@@ -138,20 +138,25 @@ def console(input_):
action="store_const",
const="summary",
dest='mode',
help="Only display the summary, not the individual "
 "test results")
 excGroup1.add_argument("-i", "--incomplete",
action="store_const",
const="incomplete",
dest='mode',
help="Only display tests that are incomplete.")
+excGroup1.add_argument("-r", "--regressions",
+  action="store_const",
+  const="regressions",
+  dest='mode',
+  help="One display tests that regressed.")
 parser.add_argument("-l", "--list",
 action="store",
 help="Use test results from a list file")
 parser.add_argument("results",
 metavar="",
 nargs="+",
 help="Space separated paths to at least one results "
  "file")
 args = parser.parse_args(unparsed)
 
diff --git a/framework/summary/console_.py b/framework/summary/console_.py
index e17a1d8..baeb6e3 100644
--- a/framework/summary/console_.py
+++ b/framework/summary/console_.py
@@ -92,24 +92,26 @@ def _print_summary(results):
 def _print_result(results, list_):
 """Takes a list of test names to print and prints the name and result."""
 for test in sorted(list_):
 print("{test}: {statuses}".format(
 test=grouptools.format(test),
 statuses=' '.join(str(r) for r in results.get_result(test
 
 
 def console(results, mode):
 """ Write summary information to the console """
-assert mode in ['summary', 'diff', 'incomplete', 'all'], mode
+assert mode in ['summary', 'diff', 'incomplete', 'regressions', 'all'], 
mode
 results = Results([backends.load(r) for r in results])
 
 # Print the name of the test and the status from each test run
 if mode == 'all':
 _print_result(results, results.names.all)
 _print_summary(results)
 elif mode == 'diff':
 _print_result(results, results.names.all_changes)
 _print_summary(results)
 elif mode == 'incomplete':
 _print_result(results, results.names.all_incomplete)
+elif mode == 'regressions':
+_print_result(results, results.names.all_regressions)
 elif mode == 'summary':
 _print_summary(results)
-- 
2.7.4

___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

Re: [Piglit] [PATCH 5/7] arb_enhanced_layouts: Multiple layout qualifiers in a single declaration tests

[Andres Gomez]

On Mon, 2016-10-24 at 11:05 +1100, Timothy Arceri wrote:
> On Sat, 2016-10-22 at 23:42 +0300, Andres Gomez wrote:
> > We already had tests for a repeated layout-qualifier-name in a single
> > layout-qualifier. Now, we also add similar tests to check across
> > multiple layout-qualifiers in the same declaration.
> > 
> > From the ARB_enhanced_layouts spec:
> > 
> > "More than one layout qualifier may appear in a single
> > declaration.
> >  Additionally, the same layout-qualifier-name can occur multiple
> > times
> >  within a layout qualifier or across multiple layout qualifiers
> > in the
> >  same declaration"
> 
> 
> I believe this is just because the spec is written against the GLSL
> 4.30 spec. I'm not so sure that enhanced layouts should add this
> support. Did you test the Nvidia and Amd binary drivers to see what
> they do?

I run the tests with the proprietary drivers in a GNU/Linux box.

NVidia and AMD are a mess with respect to this. Some pass, some not.

My conclusion is that their implementation depends on the specific
layout-qualifier-name tested. Therefore, if with, let's say
max_vertices, they don't allow duplicates with different values, they
will fail in all the cases; with duplicates in a single layout-
qualifier or among several in the same declaration, no matter
if ARB_shading_language_420pack or ARB_enhanced_layouts are supported.

If duplicates with different values are allowed for the layout-
qualifier-name tested, then the ones failing will be the negative
tests. A mess ...

Let's drop this by now. It is always easy to add more tests later.

> Also I believe we should already have negative tests for this so some
> of these tests are likely duplicates of existing tests,
> probably ARB_shading_language_420pack tests.

Oh, right, it is stupid to test for the absence in 2 different places.
Good catch.

-- 
Br,

Andres
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

Re: [Piglit] [PATCH 4/7] arb_shading_language_420pack: More multiple layout qualifiers in a single declaration tests

[Andres Gomez]

On Mon, 2016-10-24 at 11:44 +1100, Timothy Arceri wrote:
> Reviewed-by: Timothy Arceri 
> 
> On Sat, 2016-10-22 at 23:42 +0300, Andres Gomez wrote:

[snip]

> > diff --git a/tests/spec/arb_shading_language_420pack/compiler/layout-
> > qualifiers/multiple-local_size-in-single-declaration-mismatch.comp
> > b/tests/spec/arb_shading_language_420pack/compiler/layout-
> > qualifiers/multiple-local_size-in-single-declaration-mismatch.comp
> > new file mode 100644
> > index 000..b4f49f0
> > --- /dev/null
> > +++ b/tests/spec/arb_shading_language_420pack/compiler/layout-
> > qualifiers/multiple-local_size-in-single-declaration-mismatch.comp
> > @@ -0,0 +1,29 @@
> > +// [config]
> > +// expect_result: fail
> > +// glsl_version: 1.50
> > +// require_extensions: GL_ARB_shading_language_420pack
> > GL_ARB_compute_shader
> > +// check_link: true
> 
> 
> I think you need to remove this since the spec says it is a compile
> time error.

Good catch!

The spec specifies compile-time error if the mismatch is in the same
shader and link-time error if the mismatch is among shaders in the same
program.

I'll update this (and other tests that I've added in relation with
local_size).

Thanks for the review!

-- 
Br,

Andres
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

Re: [Piglit] [PATCH 1/7] glsl-1.50: Add GS output layout qualifier redeclaration test

[Andres Gomez]

On Mon, 2016-10-24 at 11:15 +1100, Timothy Arceri wrote:
> On Sat, 2016-10-22 at 23:42 +0300, Andres Gomez wrote:
> > Section 4.3.8.2 (Output Layout Qualifiers) of the GLSL 1.50 spec
> > says:
> > 
> > "All geometry shader output layout declarations in a program must
> > declare the
> >  same layout and same value for max_vertices."
> > 
> > Signed-off-by: Andres Gomez 
> > ---
> >  ...ut-declaration-consistent-with-prev-layout.geom | 37
> > ++
> >  1 file changed, 37 insertions(+)
> >  create mode 100644 tests/spec/glsl-1.50/compiler/layout-out-
> > declaration-consistent-with-prev-layout.geom
> 
> 
> We have a negative test for this:
> 
> tests/spec/glsl-1.50/compiler/layout-only-one-out-declaration-per-
> program-max-verts.geom
> 
> It would probably be nice to have similair names maybe just:
> 
> layout-only-one-out-declaration-per-program-max-verts2.geom

What about renaming the existent to:

layout-only-one-out-declaration-per-program-max-verts-mismatch.geom

And the new one to:

layout-only-one-out-declaration-per-program-max-verts.geom

That would be consistent with many existing and the rest of the newly
added tests.

> Otherwise:
> 
> Reviewed-by: Timothy Arceri 

Thanks for the review!

-- 
Br,

Andres
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH] util: implement eventloop for wayland platform

[Tapani Pälli]

Signed-off-by: Tapani Pälli 
---
 .../util/piglit-framework-gl/piglit_wl_framework.c | 137 +++--
 1 file changed, 129 insertions(+), 8 deletions(-)

diff --git a/tests/util/piglit-framework-gl/piglit_wl_framework.c 
b/tests/util/piglit-framework-gl/piglit_wl_framework.c
index 78ffea6..6b832f6 100644
--- a/tests/util/piglit-framework-gl/piglit_wl_framework.c
+++ b/tests/util/piglit-framework-gl/piglit_wl_framework.c
@@ -24,14 +24,136 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "piglit-util-gl.h"
 #include "piglit_wl_framework.h"
 
+#include 
+#include 
+#include 
+
+static struct wl_seat *seat = NULL;
+static struct wl_keyboard *keyboard = NULL;
+
+static void keymap(void *data,
+   struct wl_keyboard *wl_keyboard,
+   uint32_t format,
+   int32_t fd,
+   uint32_t size)
+{
+}
+
+static void enter(void *data,
+   struct wl_keyboard *wl_keyboard,
+   uint32_t serial,
+   struct wl_surface *surface,
+   struct wl_array *keys)
+{
+}
+
+static void leave(void *data,
+   struct wl_keyboard *wl_keyboard,
+   uint32_t serial,
+   struct wl_surface *surface)
+{
+}
+
+static void key(void *data,
+   struct wl_keyboard *wl_keyboard,
+   uint32_t serial,
+   uint32_t time,
+   uint32_t key,
+   uint32_t state)
+{
+   struct piglit_winsys_framework *winsys_fw =
+   (struct piglit_winsys_framework *) data;
+
+   winsys_fw->need_redisplay = true;
+
+   switch (key) {
+   case KEY_ESC:
+if (winsys_fw->user_keyboard_func)
+   winsys_fw->user_keyboard_func(27, 0, 0);
+   break;
+   }
+}
+
+static void modifiers(void *data,
+   struct wl_keyboard *wl_keyboard,
+   uint32_t serial,
+   uint32_t mods_depressed,
+   uint32_t mods_latched,
+   uint32_t mods_locked,
+   uint32_t group)
+{
+}
+
+static const struct wl_keyboard_listener keyboard_listener = {
+   keymap,
+   enter,
+   leave,
+   key,
+   modifiers
+};
+
+static void
+process_events(struct wl_display *dpy)
+{
+   struct pollfd fds = {
+   .fd = wl_display_get_fd(dpy),
+   .events = POLLIN,
+   .revents = 0
+   };
+
+   wl_display_sync(dpy);
+
+   while (1) {
+   poll(, 1, -1);
+   if (wl_display_dispatch(dpy) < 0)
+   break;
+   }
+}
+
+static void global(void *data,
+   struct wl_registry *wl_registry,
+   uint32_t name,
+   const char *interface,
+   uint32_t version)
+{
+   if (strcmp(interface, "wl_seat") != 0)
+   return;
+
+   seat = wl_registry_bind(wl_registry, name, _seat_interface, 1);
+   keyboard = wl_seat_get_keyboard(seat);
+
+   if (keyboard)
+   wl_keyboard_add_listener(keyboard, _listener, data);
+}
+
+static void global_remove(void *data,
+   struct wl_registry *wl_registry,
+   uint32_t name)
+{
+}
+
+static const struct wl_registry_listener registry_listener = {
+   global,
+   global_remove
+};
+
 static void
 enter_event_loop(struct piglit_winsys_framework *winsys_fw)
 {
-   const struct piglit_gl_test_config *test_config = 
winsys_fw->wfl_fw.gl_fw.test_config;
+   const struct piglit_gl_test_config *test_config =
+   winsys_fw->wfl_fw.gl_fw.test_config;
+   union waffle_native_window *n_window =
+   waffle_window_get_native(winsys_fw->wfl_fw.window);
+   struct wl_display *dpy = n_window->wayland->display.wl_display;
+   struct wl_registry *registry = wl_display_get_registry(dpy);
+   enum piglit_result result = PIGLIT_PASS;
+
+   wl_registry_add_listener(registry, _listener, winsys_fw);
 
/* The Wayland window fails to appear on the first call to
 * swapBuffers (which occured in display_cb above). This is
@@ -40,14 +162,13 @@ enter_event_loop(struct piglit_winsys_framework *winsys_fw)
 * redraw as a workaround.
 */
if (test_config->display)
-   test_config->display();
+   result = test_config->display();
 
-   /* FINISHME: Write event loop for Wayland.
-*
-* Until we have proper Wayland support, give the user enough time
-* to view the window by sleeping.
-*/
-   sleep(8);
+   /* Do not proceed to event loop in case of piglit_automatic. */
+   if (piglit_automatic)
+   piglit_report_result(result);
+
+   process_events(dpy);
 }
 
 static void
-- 
2.7.4

___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

Re: [Piglit] [PATCH] gles-es-1.00: add linker test to check default precision qualifier redeclaration

[Samuel Iglesias Gonsálvez]

On 21/10/16 05:55, Timothy Arceri wrote:
> On Thu, 2016-10-20 at 12:37 +0200, Samuel Iglesias Gonsálvez wrote:
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97804
>> Signed-off-by: Samuel Iglesias Gonsálvez 
>> ---
>>  ...t-precision-qualifier-redeclaration.shader_test | 34
>> ++
>>  1 file changed, 34 insertions(+)
>>  create mode 100644 tests/spec/glsl-es-1.00/linker/glsl-default-
>> precision-qualifier-redeclaration.shader_test
>>
>> diff --git a/tests/spec/glsl-es-1.00/linker/glsl-default-precision-
>> qualifier-redeclaration.shader_test b/tests/spec/glsl-es-
>> 1.00/linker/glsl-default-precision-qualifier-
>> redeclaration.shader_test
>> new file mode 100644
>> index 000..510ba20
>> --- /dev/null
>> +++ b/tests/spec/glsl-es-1.00/linker/glsl-default-precision-
>> qualifier-redeclaration.shader_test
>> @@ -0,0 +1,34 @@
>> +#
>> +# Test that vertex shader defaults to highp (only fragment shader
>> +# needs to specify float precision) and that later fragment shader
>> +# highp precision declaration overrides earlier mediump declaration.
>> +#
>> +[require]
>> +GL ES >= 2.0
>> +GLSL ES >= 1.00
>> +
>> +[vertex shader]
>> +
>> +uniform float a;
>> +
>> +void main(void)
>> +{
>> +gl_Position = vec4(a);
>> +}
>> +
>> +[fragment shader]
>> +#ifdef GL_ES
>> +precision mediump float;
>> +#endif
>> +#ifdef GL_ES
>> +precision highp float;
>> +#endif
> 
> The test is fine but I believe we need more tests for the scoping
> rules.
> 
> "The precision statement has the same scoping rules as variable 
> declarations. If it is declared inside a compound statement, its effect
> stops at the end of the innermost 
> statement it was declared in. Precision statements in nested scopes
> override precision statements in outer 
> scopes.

I verified these are already tested in:

tests/spec/glsl-es-1.00/compiler/precision-qualifiers/default-precision-nested-scope-0X.frag

with X from 1 to 4.

Multiple precision statements for the same basic type can
> appear inside the same scope, with later 
> statements overriding earlier statements within that scope"
> 
> This test only tests the last rule.
> 

Right.

So I think I can push this patch. Do you agree?

Sam

>> +
>> +uniform float a;
>> +
>> +void main(void)
>> +{
>> +gl_FragColor = vec4(a);
>> +}
>> +[test]
>> +link success
> 



signature.asc
Description: OpenPGP digital signature
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

Re: [Piglit] [PATCH 0/7] More duplicated layout-qualifier-names tests

[Andres Gomez]

On Mon, 2016-10-24 at 11:58 +1100, Timothy Arceri wrote:
> On Sat, 2016-10-22 at 23:42 +0300, Andres Gomez wrote:
> > ARB_shading_language_420pack
> 
> 
> You may be interested in finishing off this patch:
> 
> https://patchwork.freedesktop.org/patch/71093/
> 
> I never got around to implementing Francisco's comments that last time
> I was working on this stuff.

I'll give it a look. Thanks for noticing! ☺

-- 
Br,

Andres
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit