[Piglit] [PATCH 01/15] tests/util: Add config value to enumerate subtests

[Fabian Bieler]

I can resend to the list.
Did we reach an agreement on how piglit_run_selected_subtests should behave?

Should it
enumerate all tests and mark unselected tests PIGLIT_SKIP
or
enumerate only selected tests implicitly marking unselected tests as "not run" when compared to another test run?

I favor the second option as understand PIGLIT_SKIP means "tests ought to be run, but for some reason (missing extension, limited resources, ...) aren't" and not "test shouldn't be run to begin with (e.g. because it's not included in the current test suite)" for which I thought "not run" is the appropriate status.

Fabian

On Fri, Jan 26, 2018, at 6:53 PM, Dylan Baker wrote:
> Fabian,
> 
> When you get a chance can you send your updated version of this series to the
> list? I think this is probably a good candidate for piecemeal fixes, we can
> land the framework bits and the initial tests that are ported, then land the
> changes to the rest of the tests in manageable chunks.
> 
> Dylan
> 
> Quoting Fabian Bieler (2018-01-23 15:35:32)
> > Hello!
> > 
> > So after some consideration I think I prefer your first approach
> > (explicitly calling the enumeration function) after all.
> > 
> > First of all: Sorry for the back and forth on my part and the excess
> > work you put into this series because of it. Please don't hate me! 🙇
> > 
> > However, I think this approach will be more DRY and less WET.
> > In particular when used with the existing struct piglit_subtest and
> > piglit_run_selected_subtests.
> > 
> > I updated the series accordingly (and fixed a type-o in a docstring in
> > 5/15 and the commit message of 6/15 and a small arithmetical error in
> > 11/15).
> > 
> > The result can be found here:
> > https://github.com/fabianbieler/piglit/tree/enumerate_subtests
> > If this isn't acceptable for intermediate review, I'll post to the list,
> > too.
> > 
> > Note that I leveraged piglit_run_selected_subtests for gl-2.1-pbo,
> > fbo-incomplete, degenerate-prims and linestipple and used
> > piglit_register_subtests in fbo-storage-formats.
> > 
> > Regards (and sorry again)
> > Fabian
> > 
> > On 2018-01-23 02:22, Dylan Baker wrote:
> > > This adds a new member to the GL config struct for informing the python
> > > framework the number and order of subtests that will be run (if any). If
> > > this value is unset then no subtests are expected.
> > > ---
> > >  tests/util/piglit-framework-gl.c | 13 +++++++++++++
> > >  tests/util/piglit-framework-gl.h | 11 +++++++++++
> > >  2 files changed, 24 insertions(+)
> > > 
> > > diff --git a/tests/util/piglit-framework-gl.c b/tests/util/piglit-framework-gl.c
> > > index 1b2078d..1ce5474 100644
> > > --- a/tests/util/piglit-framework-gl.c
> > > +++ b/tests/util/piglit-framework-gl.c
> > > @@ -318,3 +318,16 @@ piglit_get_selected_tests(const char ***selected_subtests)
> > >       *selected_subtests = gl_fw->test_config->selected_subtests;
> > >       return gl_fw->test_config->num_selected_subtests;
> > >  }
> > > +
> > > +void
> > > +piglit_gl_enumerate_subtests(const struct piglit_gl_test_config *config)
> > > +{
> > > +     if (config->all_subtests) {
> > > +             printf("PIGLIT: {\"enumerate subtests\": [\"%s\"", config->all_subtests[0]);
> > > +             for (int i = 1; config->all_subtests[i]; i++) {
> > > +                     printf(", \"%s\"", config->all_subtests[i]);
> > > +             }
> > > +             printf("]}\n");
> > > +             fflush(stdout);
> > > +     }
> > > +}
> > > diff --git a/tests/util/piglit-framework-gl.h b/tests/util/piglit-framework-gl.h
> > > index 970fd55..d36c4f0 100644
> > > --- a/tests/util/piglit-framework-gl.h
> > > +++ b/tests/util/piglit-framework-gl.h
> > > @@ -213,6 +213,13 @@ struct piglit_gl_test_config {
> > >        * enum used for markin test as supporting KHR_no_error or not.
> > >        */
> > >       enum piglit_khr_no_error_support khr_no_error_support;
> > > +
> > > +     /**
> > > +      * Null terminated list of subtests to be enumerated.
> > > +      *
> > > +      * Each subtest *must* be run in the order reported by this list.
> > > +      */
> > > +     const char **all_subtests;
> > >  };
> > >  
> > >  /**
> > > @@ -246,6 +253,9 @@ void
> > >  piglit_gl_test_run(int argc, char *argv[],
> > >                  const struct piglit_gl_test_config *config);
> > >  
> > > +void
> > > +piglit_gl_enumerate_subtests(const struct piglit_gl_test_config *config);
> > > +
> > >  #ifdef __cplusplus
> > >  #  define PIGLIT_EXTERN_C_BEGIN extern "C" {
> > >  #  define PIGLIT_EXTERN_C_END   }
> > > @@ -287,6 +297,7 @@ piglit_gl_test_run(int argc, char *argv[],
> > >                  }                                                            \
> > >                                                                               \
> > >                  piglit_gl_process_args(&argc, argv, &config);                \
> > > +                piglit_gl_enumerate_subtests(&config);                       \
> > >                  piglit_gl_test_run(argc, argv, &config);                     \
> > >                                                                               \
> > >                  assert(false);                                               \
> > > 
> > > base-commit: 736496667329bf73a706aebec6f8287078df79ae
> > > 
> Email had 1 attachment:
> + signature.asc
>   1k (application/pgp-signature)