Re: [Intel-gfx] [PATCH i-g-t 1/5] Add support for subtest-specific documentation

2017-09-04 Thread Daniel Vetter
On Mon, Sep 04, 2017 at 08:42:58AM +, Szwichtenberg, Radoslaw wrote:
> On Fri, 2017-08-11 at 10:20 +0200, Daniel Vetter wrote:
> > On Thu, Aug 10, 2017 at 01:26:47PM +0300, Petri Latvala wrote:
> > > The current documentation for tests is limited to a single string per
> > > test binary. This patch adds support for documenting individual
> > > subtests.
> > > 
> > > The syntax for subtest documentation is:
> > > 
> > >    igt_document_subtest("Frob knobs to see if one of the "
> > > "crossbeams will go out of skew on the "
> > > "treadle.\n");
> > >    igt_subtest("knob-frobbing-askew")
> > >  test_frob();
> > > 
> > > or with a format string:
> > > 
> > >   for_example_loop(e) {
> > > igt_document_subtest_f("Frob %s to see if one of the "
> > >    "crossbeams will go out of skew on the "
> > >    "treadle.\n", e->readable_name);
> > > igt_subtest_f("%s-frob-askew", e->name)
> > >   test_frob(e);
> > >   }
> > 
> > I'm not sold on this layout at all. Everywhere else where we do in-line
> > code documentation it is through specially formatted comments. That gives
> > you a lot of freedom for plain-text layout, in combination with some mild
> > markup (gtk-doc for igt and rst for kernel) that result in docs which both
> > look good in the code and look good rendered.
> > 
> > This here essentially restricts you to one-liners, and a one-liner can't
> > really explain what a more complext test does.
> I second that. For many cases one-liner will do - but these more complicated
> cases are really worth the effort when documenting.

I'm definitely not against documenting more involved testcases, e.g.
kms_frontbuffer_tracking. But this RFC here very much suggests a lot more
beaurocracy documenting everything, and not really some in-depth comments
where needed.

> > If we imagine what e.g. Paulo's test documentation in
> > kms_frontbuffer_tracking.c looks like, it'll be bad.
> > 
> > I thought the test documentation that Thomas Wood worked on (no idea about
> > status) would give us the full power and expressiveness of gtkdoc, but for
> > binaries. I think that's what we want.
> > 
> > Then for testcases I think we again want to follow the inline
> > documentation style, perhaps with something like the below:
> > 
> > 
> > /**
> >  * IGT-Subtest: tests some stuff
> >  *
> >  * Longer explanation of test approach and result evalution.
> >  *
> >  * Maybe over multiple paragraphs with headlines like CAVEATS, or
> >  * explaining hw bugs and stuff
> >  */
> > igt_subtest("bla-bla-subtest")
> > 
> > 
> > There's also the question of how to associate a given doc entry with more
> > the multiple subtest names that igt_subtest_f can generate, but I guess
> > that should be solveable.
> I don't think its even worth having same text with just single words changed
> generated for every subtest if test name describes the difference (and I guess
> in many cases that is what we have now). It would be good to document such 
> tests
> in groups.

Yes, I don't think it makes much sense to document every subtest. Some are
better documented as groups, many are just plain trivial (e.g.
kms_addfb_basic), and for others it might be better to comment the exact
test approach in-line in the code.
-Daniel

> 
> Thanks,
> Radek
> > 
> > Any, in my opinion documentation has to look pleasing, both in code and
> > rendered, otherwise people will not look at it, not write it and not
> > update it. Or worse, they stick to writing full comments, because that's
> > the only thing that allows them to express what they want to document.
> > 
> > We need something much better imo than this patch series from that pov.
> > 
> > Thanks, Daniel
> > 
> > > The documentation cannot be extracted from just comments, because
> > > associating them with the correct subtest name will then require doing
> > > pattern matching in the documentation generator, for subtests where
> > > the name is generated at runtime using igt_subtest_f.
> > > 
> > > v2: Rebase, change function name in commit message to match code
> > > 
> > > v3: Fix current documentation string tracking, add missing va_end (Vinay)
> > > 
> > > v4: Fix brainfart in __igt_run_subtest
> > > 
> > > Signed-off-by: Petri Latvala 
> > > Acked-by: Leo Li 
> > > Acked-by: Arkadiusz Hiler 
> > > ---
> > >  lib/igt_aux.c  |   8 ++--
> > >  lib/igt_core.c | 149 
> > > +-
> > > ---
> > >  lib/igt_core.h |   6 ++-
> > >  3 files changed, 128 insertions(+), 35 deletions(-)
> > > 
> > > diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> > > index f428f15..d56f41f 100644
> > > --- a/lib/igt_aux.c
> > > +++ b/lib/igt_aux.c
> > > @@ -311,7 +311,7 @@ static void sig_handler(int i)
> > >   */
> > >  void igt_fork_signal_helper(void)
> > >  {
> > > - 

Re: [Intel-gfx] [PATCH i-g-t 1/5] Add support for subtest-specific documentation

2017-09-04 Thread Szwichtenberg, Radoslaw
On Fri, 2017-08-11 at 10:20 +0200, Daniel Vetter wrote:
> On Thu, Aug 10, 2017 at 01:26:47PM +0300, Petri Latvala wrote:
> > The current documentation for tests is limited to a single string per
> > test binary. This patch adds support for documenting individual
> > subtests.
> > 
> > The syntax for subtest documentation is:
> > 
> >    igt_document_subtest("Frob knobs to see if one of the "
> > "crossbeams will go out of skew on the "
> > "treadle.\n");
> >    igt_subtest("knob-frobbing-askew")
> >  test_frob();
> > 
> > or with a format string:
> > 
> >   for_example_loop(e) {
> > igt_document_subtest_f("Frob %s to see if one of the "
> >    "crossbeams will go out of skew on the "
> >    "treadle.\n", e->readable_name);
> > igt_subtest_f("%s-frob-askew", e->name)
> >   test_frob(e);
> >   }
> 
> I'm not sold on this layout at all. Everywhere else where we do in-line
> code documentation it is through specially formatted comments. That gives
> you a lot of freedom for plain-text layout, in combination with some mild
> markup (gtk-doc for igt and rst for kernel) that result in docs which both
> look good in the code and look good rendered.
> 
> This here essentially restricts you to one-liners, and a one-liner can't
> really explain what a more complext test does.
I second that. For many cases one-liner will do - but these more complicated
cases are really worth the effort when documenting.
> 
> If we imagine what e.g. Paulo's test documentation in
> kms_frontbuffer_tracking.c looks like, it'll be bad.
> 
> I thought the test documentation that Thomas Wood worked on (no idea about
> status) would give us the full power and expressiveness of gtkdoc, but for
> binaries. I think that's what we want.
> 
> Then for testcases I think we again want to follow the inline
> documentation style, perhaps with something like the below:
> 
> 
>   /**
>* IGT-Subtest: tests some stuff
>*
>* Longer explanation of test approach and result evalution.
>*
>* Maybe over multiple paragraphs with headlines like CAVEATS, or
>* explaining hw bugs and stuff
>*/
>   igt_subtest("bla-bla-subtest")
> 
> 
> There's also the question of how to associate a given doc entry with more
> the multiple subtest names that igt_subtest_f can generate, but I guess
> that should be solveable.
I don't think its even worth having same text with just single words changed
generated for every subtest if test name describes the difference (and I guess
in many cases that is what we have now). It would be good to document such tests
in groups.

Thanks,
Radek
> 
> Any, in my opinion documentation has to look pleasing, both in code and
> rendered, otherwise people will not look at it, not write it and not
> update it. Or worse, they stick to writing full comments, because that's
> the only thing that allows them to express what they want to document.
> 
> We need something much better imo than this patch series from that pov.
> 
> Thanks, Daniel
> 
> > The documentation cannot be extracted from just comments, because
> > associating them with the correct subtest name will then require doing
> > pattern matching in the documentation generator, for subtests where
> > the name is generated at runtime using igt_subtest_f.
> > 
> > v2: Rebase, change function name in commit message to match code
> > 
> > v3: Fix current documentation string tracking, add missing va_end (Vinay)
> > 
> > v4: Fix brainfart in __igt_run_subtest
> > 
> > Signed-off-by: Petri Latvala 
> > Acked-by: Leo Li 
> > Acked-by: Arkadiusz Hiler 
> > ---
> >  lib/igt_aux.c  |   8 ++--
> >  lib/igt_core.c | 149 +-
> > ---
> >  lib/igt_core.h |   6 ++-
> >  3 files changed, 128 insertions(+), 35 deletions(-)
> > 
> > diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> > index f428f15..d56f41f 100644
> > --- a/lib/igt_aux.c
> > +++ b/lib/igt_aux.c
> > @@ -311,7 +311,7 @@ static void sig_handler(int i)
> >   */
> >  void igt_fork_signal_helper(void)
> >  {
> > -   if (igt_only_list_subtests())
> > +   if (igt_only_collect_data())
> >     return;
> >  
> >     /* We pick SIGCONT as it is a "safe" signal - if we send SIGCONT to
> > @@ -344,7 +344,7 @@ void igt_fork_signal_helper(void)
> >   */
> >  void igt_stop_signal_helper(void)
> >  {
> > -   if (igt_only_list_subtests())
> > +   if (igt_only_collect_data())
> >     return;
> >  
> >     igt_stop_helper(_helper);
> > @@ -375,7 +375,7 @@ static void __attribute__((noreturn))
> > shrink_helper_process(int fd, pid_t pid)
> >   */
> >  void igt_fork_shrink_helper(int drm_fd)
> >  {
> > -   assert(!igt_only_list_subtests());
> > +   assert(!igt_only_collect_data());
> >     igt_require(igt_drop_caches_has(drm_fd, DROP_SHRINK_ALL));
> >     

Re: [Intel-gfx] [PATCH i-g-t 1/5] Add support for subtest-specific documentation

2017-08-11 Thread Daniel Vetter
On Thu, Aug 10, 2017 at 01:26:47PM +0300, Petri Latvala wrote:
> The current documentation for tests is limited to a single string per
> test binary. This patch adds support for documenting individual
> subtests.
> 
> The syntax for subtest documentation is:
> 
>igt_document_subtest("Frob knobs to see if one of the "
> "crossbeams will go out of skew on the "
> "treadle.\n");
>igt_subtest("knob-frobbing-askew")
>  test_frob();
> 
> or with a format string:
> 
>   for_example_loop(e) {
> igt_document_subtest_f("Frob %s to see if one of the "
>"crossbeams will go out of skew on the "
>"treadle.\n", e->readable_name);
> igt_subtest_f("%s-frob-askew", e->name)
>   test_frob(e);
>   }

I'm not sold on this layout at all. Everywhere else where we do in-line
code documentation it is through specially formatted comments. That gives
you a lot of freedom for plain-text layout, in combination with some mild
markup (gtk-doc for igt and rst for kernel) that result in docs which both
look good in the code and look good rendered.

This here essentially restricts you to one-liners, and a one-liner can't
really explain what a more complext test does.

If we imagine what e.g. Paulo's test documentation in
kms_frontbuffer_tracking.c looks like, it'll be bad.

I thought the test documentation that Thomas Wood worked on (no idea about
status) would give us the full power and expressiveness of gtkdoc, but for
binaries. I think that's what we want.

Then for testcases I think we again want to follow the inline
documentation style, perhaps with something like the below:


/**
 * IGT-Subtest: tests some stuff
 *
 * Longer explanation of test approach and result evalution.
 *
 * Maybe over multiple paragraphs with headlines like CAVEATS, or
 * explaining hw bugs and stuff
 */
igt_subtest("bla-bla-subtest")


There's also the question of how to associate a given doc entry with more
the multiple subtest names that igt_subtest_f can generate, but I guess
that should be solveable.

Any, in my opinion documentation has to look pleasing, both in code and
rendered, otherwise people will not look at it, not write it and not
update it. Or worse, they stick to writing full comments, because that's
the only thing that allows them to express what they want to document.

We need something much better imo than this patch series from that pov.

Thanks, Daniel

> The documentation cannot be extracted from just comments, because
> associating them with the correct subtest name will then require doing
> pattern matching in the documentation generator, for subtests where
> the name is generated at runtime using igt_subtest_f.
> 
> v2: Rebase, change function name in commit message to match code
> 
> v3: Fix current documentation string tracking, add missing va_end (Vinay)
> 
> v4: Fix brainfart in __igt_run_subtest
> 
> Signed-off-by: Petri Latvala 
> Acked-by: Leo Li 
> Acked-by: Arkadiusz Hiler 
> ---
>  lib/igt_aux.c  |   8 ++--
>  lib/igt_core.c | 149 
> +
>  lib/igt_core.h |   6 ++-
>  3 files changed, 128 insertions(+), 35 deletions(-)
> 
> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> index f428f15..d56f41f 100644
> --- a/lib/igt_aux.c
> +++ b/lib/igt_aux.c
> @@ -311,7 +311,7 @@ static void sig_handler(int i)
>   */
>  void igt_fork_signal_helper(void)
>  {
> - if (igt_only_list_subtests())
> + if (igt_only_collect_data())
>   return;
>  
>   /* We pick SIGCONT as it is a "safe" signal - if we send SIGCONT to
> @@ -344,7 +344,7 @@ void igt_fork_signal_helper(void)
>   */
>  void igt_stop_signal_helper(void)
>  {
> - if (igt_only_list_subtests())
> + if (igt_only_collect_data())
>   return;
>  
>   igt_stop_helper(_helper);
> @@ -375,7 +375,7 @@ static void __attribute__((noreturn)) 
> shrink_helper_process(int fd, pid_t pid)
>   */
>  void igt_fork_shrink_helper(int drm_fd)
>  {
> - assert(!igt_only_list_subtests());
> + assert(!igt_only_collect_data());
>   igt_require(igt_drop_caches_has(drm_fd, DROP_SHRINK_ALL));
>   igt_fork_helper(_helper)
>   shrink_helper_process(drm_fd, getppid());
> @@ -473,7 +473,7 @@ void igt_stop_hang_detector(void)
>  #else
>  void igt_fork_hang_detector(int fd)
>  {
> - if (igt_only_list_subtests())
> + if (igt_only_collect_data())
>   return;
>  }
>  
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index c0488e9..f126ef8 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -99,7 +99,7 @@
>   *
>   * To allow this i-g-t provides #igt_fixture code blocks for setup code 
> outside
>   * of subtests and automatically skips the subtest code blocks themselves. 
> For
> - * special cases 

Re: [Intel-gfx] [PATCH i-g-t 1/5] Add support for subtest-specific documentation

2017-08-10 Thread Belgaumkar, Vinay



On 8/10/2017 3:26 AM, Petri Latvala wrote:

The current documentation for tests is limited to a single string per
test binary. This patch adds support for documenting individual
subtests.

The syntax for subtest documentation is:

igt_document_subtest("Frob knobs to see if one of the "
 "crossbeams will go out of skew on the "
 "treadle.\n");
igt_subtest("knob-frobbing-askew")
  test_frob();

or with a format string:

   for_example_loop(e) {
 igt_document_subtest_f("Frob %s to see if one of the "
"crossbeams will go out of skew on the "
"treadle.\n", e->readable_name);
 igt_subtest_f("%s-frob-askew", e->name)
   test_frob(e);
   }

The documentation cannot be extracted from just comments, because
associating them with the correct subtest name will then require doing
pattern matching in the documentation generator, for subtests where
the name is generated at runtime using igt_subtest_f.

v2: Rebase, change function name in commit message to match code

v3: Fix current documentation string tracking, add missing va_end (Vinay)

v4: Fix brainfart in __igt_run_subtest

Signed-off-by: Petri Latvala 
Acked-by: Leo Li 
Acked-by: Arkadiusz Hiler 
---
  lib/igt_aux.c  |   8 ++--
  lib/igt_core.c | 149 +
  lib/igt_core.h |   6 ++-
  3 files changed, 128 insertions(+), 35 deletions(-)

diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index f428f15..d56f41f 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -311,7 +311,7 @@ static void sig_handler(int i)
   */
  void igt_fork_signal_helper(void)
  {
-   if (igt_only_list_subtests())
+   if (igt_only_collect_data())
return;
  
  	/* We pick SIGCONT as it is a "safe" signal - if we send SIGCONT to

@@ -344,7 +344,7 @@ void igt_fork_signal_helper(void)
   */
  void igt_stop_signal_helper(void)
  {
-   if (igt_only_list_subtests())
+   if (igt_only_collect_data())
return;
  
  	igt_stop_helper(_helper);

@@ -375,7 +375,7 @@ static void __attribute__((noreturn)) 
shrink_helper_process(int fd, pid_t pid)
   */
  void igt_fork_shrink_helper(int drm_fd)
  {
-   assert(!igt_only_list_subtests());
+   assert(!igt_only_collect_data());
igt_require(igt_drop_caches_has(drm_fd, DROP_SHRINK_ALL));
igt_fork_helper(_helper)
shrink_helper_process(drm_fd, getppid());
@@ -473,7 +473,7 @@ void igt_stop_hang_detector(void)
  #else
  void igt_fork_hang_detector(int fd)
  {
-   if (igt_only_list_subtests())
+   if (igt_only_collect_data())
return;
  }
  
diff --git a/lib/igt_core.c b/lib/igt_core.c

index c0488e9..f126ef8 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -99,7 +99,7 @@
   *
   * To allow this i-g-t provides #igt_fixture code blocks for setup code 
outside
   * of subtests and automatically skips the subtest code blocks themselves. For
- * special cases igt_only_list_subtests() is also provided. For setup code only
+ * special cases igt_only_collect_data() is also provided. For setup code only
   * shared by a group of subtest encapsulate the #igt_fixture block and all the
   * subtestest in a #igt_subtest_group block.
   *
@@ -253,9 +253,9 @@ static unsigned int exit_handler_count;
  const char *igt_interactive_debug;
  
  /* subtests helpers */

-static bool list_subtests = false;
-static char *run_single_subtest = NULL;
-static bool run_single_subtest_found = false;
+static char *single_subtest = NULL;
+static bool single_subtest_found = false;
+static char *current_subtest_documentation = NULL;
  static const char *in_subtest = NULL;
  static struct timespec subtest_time;
  static clockid_t igt_clock = (clockid_t)-1;
@@ -265,6 +265,13 @@ static bool in_atexit_handler = false;
  static enum {
CONT = 0, SKIP, FAIL
  } skip_subtests_henceforth = CONT;
+static enum {
+   EXECUTE_ALL,
+   EXECUTE_SINGLE,
+   LIST_SUBTESTS,
+   DOCUMENT,
+   DOCUMENT_SINGLE
+} runmode = EXECUTE_ALL;
  
  bool __igt_plain_output = false;
  
@@ -277,6 +284,8 @@ bool test_child;

  enum {
   OPT_LIST_SUBTESTS,
   OPT_RUN_SUBTEST,
+ OPT_DOC_SUBTESTS,
+ OPT_DOC_SINGLE_SUBTEST,
   OPT_DESCRIPTION,
   OPT_DEBUG,
   OPT_INTERACTIVE_DEBUG,
@@ -469,7 +478,7 @@ bool __igt_fixture(void)
  {
assert(!in_fixture);
  
-	if (igt_only_list_subtests())

+   if (igt_only_collect_data())
return false;
  
  	if (skip_subtests_henceforth)

@@ -563,7 +572,7 @@ static void low_mem_killer_disable(bool disable)
  bool igt_exit_called;
  static void common_exit_handler(int sig)
  {
-   if (!igt_only_list_subtests()) {
+   if (!igt_only_collect_data()) {
low_mem_killer_disable(false);
kick_fbcon(true);
}
@@ -583,7 +592,7 @@ static void print_version(void)
  {
  

[Intel-gfx] [PATCH i-g-t 1/5] Add support for subtest-specific documentation

2017-08-10 Thread Petri Latvala
The current documentation for tests is limited to a single string per
test binary. This patch adds support for documenting individual
subtests.

The syntax for subtest documentation is:

   igt_document_subtest("Frob knobs to see if one of the "
"crossbeams will go out of skew on the "
"treadle.\n");
   igt_subtest("knob-frobbing-askew")
 test_frob();

or with a format string:

  for_example_loop(e) {
igt_document_subtest_f("Frob %s to see if one of the "
   "crossbeams will go out of skew on the "
   "treadle.\n", e->readable_name);
igt_subtest_f("%s-frob-askew", e->name)
  test_frob(e);
  }

The documentation cannot be extracted from just comments, because
associating them with the correct subtest name will then require doing
pattern matching in the documentation generator, for subtests where
the name is generated at runtime using igt_subtest_f.

v2: Rebase, change function name in commit message to match code

v3: Fix current documentation string tracking, add missing va_end (Vinay)

v4: Fix brainfart in __igt_run_subtest

Signed-off-by: Petri Latvala 
Acked-by: Leo Li 
Acked-by: Arkadiusz Hiler 
---
 lib/igt_aux.c  |   8 ++--
 lib/igt_core.c | 149 +
 lib/igt_core.h |   6 ++-
 3 files changed, 128 insertions(+), 35 deletions(-)

diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index f428f15..d56f41f 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -311,7 +311,7 @@ static void sig_handler(int i)
  */
 void igt_fork_signal_helper(void)
 {
-   if (igt_only_list_subtests())
+   if (igt_only_collect_data())
return;
 
/* We pick SIGCONT as it is a "safe" signal - if we send SIGCONT to
@@ -344,7 +344,7 @@ void igt_fork_signal_helper(void)
  */
 void igt_stop_signal_helper(void)
 {
-   if (igt_only_list_subtests())
+   if (igt_only_collect_data())
return;
 
igt_stop_helper(_helper);
@@ -375,7 +375,7 @@ static void __attribute__((noreturn)) 
shrink_helper_process(int fd, pid_t pid)
  */
 void igt_fork_shrink_helper(int drm_fd)
 {
-   assert(!igt_only_list_subtests());
+   assert(!igt_only_collect_data());
igt_require(igt_drop_caches_has(drm_fd, DROP_SHRINK_ALL));
igt_fork_helper(_helper)
shrink_helper_process(drm_fd, getppid());
@@ -473,7 +473,7 @@ void igt_stop_hang_detector(void)
 #else
 void igt_fork_hang_detector(int fd)
 {
-   if (igt_only_list_subtests())
+   if (igt_only_collect_data())
return;
 }
 
diff --git a/lib/igt_core.c b/lib/igt_core.c
index c0488e9..f126ef8 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -99,7 +99,7 @@
  *
  * To allow this i-g-t provides #igt_fixture code blocks for setup code outside
  * of subtests and automatically skips the subtest code blocks themselves. For
- * special cases igt_only_list_subtests() is also provided. For setup code only
+ * special cases igt_only_collect_data() is also provided. For setup code only
  * shared by a group of subtest encapsulate the #igt_fixture block and all the
  * subtestest in a #igt_subtest_group block.
  *
@@ -253,9 +253,9 @@ static unsigned int exit_handler_count;
 const char *igt_interactive_debug;
 
 /* subtests helpers */
-static bool list_subtests = false;
-static char *run_single_subtest = NULL;
-static bool run_single_subtest_found = false;
+static char *single_subtest = NULL;
+static bool single_subtest_found = false;
+static char *current_subtest_documentation = NULL;
 static const char *in_subtest = NULL;
 static struct timespec subtest_time;
 static clockid_t igt_clock = (clockid_t)-1;
@@ -265,6 +265,13 @@ static bool in_atexit_handler = false;
 static enum {
CONT = 0, SKIP, FAIL
 } skip_subtests_henceforth = CONT;
+static enum {
+   EXECUTE_ALL,
+   EXECUTE_SINGLE,
+   LIST_SUBTESTS,
+   DOCUMENT,
+   DOCUMENT_SINGLE
+} runmode = EXECUTE_ALL;
 
 bool __igt_plain_output = false;
 
@@ -277,6 +284,8 @@ bool test_child;
 enum {
  OPT_LIST_SUBTESTS,
  OPT_RUN_SUBTEST,
+ OPT_DOC_SUBTESTS,
+ OPT_DOC_SINGLE_SUBTEST,
  OPT_DESCRIPTION,
  OPT_DEBUG,
  OPT_INTERACTIVE_DEBUG,
@@ -469,7 +478,7 @@ bool __igt_fixture(void)
 {
assert(!in_fixture);
 
-   if (igt_only_list_subtests())
+   if (igt_only_collect_data())
return false;
 
if (skip_subtests_henceforth)
@@ -563,7 +572,7 @@ static void low_mem_killer_disable(bool disable)
 bool igt_exit_called;
 static void common_exit_handler(int sig)
 {
-   if (!igt_only_list_subtests()) {
+   if (!igt_only_collect_data()) {
low_mem_killer_disable(false);
kick_fbcon(true);
}
@@ -583,7 +592,7 @@ static void print_version(void)
 {
struct utsname uts;
 
-   if (list_subtests)
+   if (igt_only_collect_data())

[Intel-gfx] [PATCH i-g-t 1/5] Add support for subtest-specific documentation

2017-08-10 Thread Petri Latvala
The current documentation for tests is limited to a single string per
test binary. This patch adds support for documenting individual
subtests.

The syntax for subtest documentation is:

   igt_document_subtest("Frob knobs to see if one of the "
"crossbeams will go out of skew on the "
"treadle.\n");
   igt_subtest("knob-frobbing-askew")
 test_frob();

or with a format string:

  for_example_loop(e) {
igt_document_subtest_f("Frob %s to see if one of the "
   "crossbeams will go out of skew on the "
   "treadle.\n", e->readable_name);
igt_subtest_f("%s-frob-askew", e->name)
  test_frob(e);
  }

The documentation cannot be extracted from just comments, because
associating them with the correct subtest name will then require doing
pattern matching in the documentation generator, for subtests where
the name is generated at runtime using igt_subtest_f.

v2: Rebase, change function name in commit message to match code

v3: Fix current documentation string tracking, add missing va_end (Vinay)

Signed-off-by: Petri Latvala 
Acked-by: Leo Li 
Acked-by: Arkadiusz Hiler 
---
 lib/igt_aux.c  |   8 ++--
 lib/igt_core.c | 148 +
 lib/igt_core.h |   6 ++-
 3 files changed, 127 insertions(+), 35 deletions(-)

diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index f428f15..d56f41f 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -311,7 +311,7 @@ static void sig_handler(int i)
  */
 void igt_fork_signal_helper(void)
 {
-   if (igt_only_list_subtests())
+   if (igt_only_collect_data())
return;
 
/* We pick SIGCONT as it is a "safe" signal - if we send SIGCONT to
@@ -344,7 +344,7 @@ void igt_fork_signal_helper(void)
  */
 void igt_stop_signal_helper(void)
 {
-   if (igt_only_list_subtests())
+   if (igt_only_collect_data())
return;
 
igt_stop_helper(_helper);
@@ -375,7 +375,7 @@ static void __attribute__((noreturn)) 
shrink_helper_process(int fd, pid_t pid)
  */
 void igt_fork_shrink_helper(int drm_fd)
 {
-   assert(!igt_only_list_subtests());
+   assert(!igt_only_collect_data());
igt_require(igt_drop_caches_has(drm_fd, DROP_SHRINK_ALL));
igt_fork_helper(_helper)
shrink_helper_process(drm_fd, getppid());
@@ -473,7 +473,7 @@ void igt_stop_hang_detector(void)
 #else
 void igt_fork_hang_detector(int fd)
 {
-   if (igt_only_list_subtests())
+   if (igt_only_collect_data())
return;
 }
 
diff --git a/lib/igt_core.c b/lib/igt_core.c
index c0488e9..7da9340 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -99,7 +99,7 @@
  *
  * To allow this i-g-t provides #igt_fixture code blocks for setup code outside
  * of subtests and automatically skips the subtest code blocks themselves. For
- * special cases igt_only_list_subtests() is also provided. For setup code only
+ * special cases igt_only_collect_data() is also provided. For setup code only
  * shared by a group of subtest encapsulate the #igt_fixture block and all the
  * subtestest in a #igt_subtest_group block.
  *
@@ -253,9 +253,9 @@ static unsigned int exit_handler_count;
 const char *igt_interactive_debug;
 
 /* subtests helpers */
-static bool list_subtests = false;
-static char *run_single_subtest = NULL;
-static bool run_single_subtest_found = false;
+static char *single_subtest = NULL;
+static bool single_subtest_found = false;
+static char *current_subtest_documentation = NULL;
 static const char *in_subtest = NULL;
 static struct timespec subtest_time;
 static clockid_t igt_clock = (clockid_t)-1;
@@ -265,6 +265,13 @@ static bool in_atexit_handler = false;
 static enum {
CONT = 0, SKIP, FAIL
 } skip_subtests_henceforth = CONT;
+static enum {
+   EXECUTE_ALL,
+   EXECUTE_SINGLE,
+   LIST_SUBTESTS,
+   DOCUMENT,
+   DOCUMENT_SINGLE
+} runmode = EXECUTE_ALL;
 
 bool __igt_plain_output = false;
 
@@ -277,6 +284,8 @@ bool test_child;
 enum {
  OPT_LIST_SUBTESTS,
  OPT_RUN_SUBTEST,
+ OPT_DOC_SUBTESTS,
+ OPT_DOC_SINGLE_SUBTEST,
  OPT_DESCRIPTION,
  OPT_DEBUG,
  OPT_INTERACTIVE_DEBUG,
@@ -469,7 +478,7 @@ bool __igt_fixture(void)
 {
assert(!in_fixture);
 
-   if (igt_only_list_subtests())
+   if (igt_only_collect_data())
return false;
 
if (skip_subtests_henceforth)
@@ -563,7 +572,7 @@ static void low_mem_killer_disable(bool disable)
 bool igt_exit_called;
 static void common_exit_handler(int sig)
 {
-   if (!igt_only_list_subtests()) {
+   if (!igt_only_collect_data()) {
low_mem_killer_disable(false);
kick_fbcon(true);
}
@@ -583,7 +592,7 @@ static void print_version(void)
 {
struct utsname uts;
 
-   if (list_subtests)
+   if (igt_only_collect_data())
return;
 
uname();