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

2017-08-10 Thread Petri Latvala
On Wed, Aug 09, 2017 at 01:18:55PM -0700, Belgaumkar, Vinay wrote:
> > @@ -710,6 +721,8 @@ static int common_init(int *argc, char **argv,
> > static struct option long_options[] = {
> > {"list-subtests", 0, 0, OPT_LIST_SUBTESTS},
> > {"run-subtest", 1, 0, OPT_RUN_SUBTEST},
> > +   {"document-all-subtests", 0, 0, OPT_DOC_SUBTESTS},
> > +   {"document-subtest", 1, 0, OPT_DOC_SINGLE_SUBTEST},
> 
> Can we add these options to the README file as well?
>


Sending another series soon with README updated.


> > {"help-description", 0, 0, OPT_DESCRIPTION},
> > {"debug", optional_argument, 0, OPT_DEBUG},
> > {"interactive-debug", optional_argument, 0, 
> > OPT_INTERACTIVE_DEBUG},
> > @@ -800,12 +813,24 @@ static int common_init(int *argc, char **argv,
> > igt_log_domain_filter = strdup(optarg);
> > break;
> > case OPT_LIST_SUBTESTS:
> > -   if (!run_single_subtest)
> > -   list_subtests = true;
> > +   if (runmode == EXECUTE_ALL)
> > +   runmode = LIST_SUBTESTS;
> > break;
> > case OPT_RUN_SUBTEST:
> > -   if (!list_subtests)
> > -   run_single_subtest = strdup(optarg);
> > +   if (runmode == EXECUTE_ALL) {
> > +   runmode = EXECUTE_SINGLE;
> > +   single_subtest = strdup(optarg);
> > +   }
> > +   break;
> > +   case OPT_DOC_SUBTESTS:
> > +   if (runmode == EXECUTE_ALL)
> > +   runmode = DOCUMENT;
> > +   break;
> > +   case OPT_DOC_SINGLE_SUBTEST:
> > +   if (runmode == EXECUTE_ALL) {
> > +   runmode = DOCUMENT_SINGLE;
> > +   single_subtest = strdup(optarg);
> > +   }
> > break;
> > case OPT_DESCRIPTION:
> > print_test_description();
> > @@ -837,11 +862,11 @@ out:
> > /* exit immediately if this test has no subtests and a subtest or the
> >  * list of subtests has been requested */
> > if (!test_with_subtests) {
> > -   if (run_single_subtest) {
> > -   igt_warn("Unknown subtest: %s\n", run_single_subtest);
> > +   if (runmode == EXECUTE_SINGLE || runmode == DOCUMENT_SINGLE) {
> > +   igt_warn("Unknown subtest: %s\n", single_subtest);
> > exit(IGT_EXIT_INVALID);
> > }
> > -   if (list_subtests)
> > +   if (runmode == LIST_SUBTESTS || runmode == DOCUMENT)
> > exit(IGT_EXIT_INVALID);
> > }
> > @@ -849,7 +874,7 @@ out:
> > /* exit with no error for -h/--help */
> > exit(ret == -1 ? 0 : IGT_EXIT_INVALID);
> > -   if (!list_subtests) {
> > +   if (!igt_only_collect_data()) {
> > kick_fbcon(false);
> > kmsg(KERN_INFO "[IGT] %s: executing\n", command_str);
> > print_version();
> > @@ -957,16 +982,37 @@ bool __igt_run_subtest(const char *subtest_name)
> > igt_exit();
> > }
> > -   if (list_subtests) {
> > +   if (runmode == LIST_SUBTESTS) {
> > printf("%s\n", subtest_name);
> > return false;
> > }
> > -   if (run_single_subtest) {
> > -   if (uwildmat(subtest_name, run_single_subtest) == 0)
> > +   if (runmode == DOCUMENT) {
> > +   if (current_subtest_documentation) {
> > +   printf("%s:\n\n", subtest_name);
> > +   printf("%s", current_subtest_documentation);
> > +   free(current_subtest_documentation);
> > +   current_subtest_documentation = NULL;
> > +   }
> > +   return false;
> > +   }
> > +
> > +   if (runmode == EXECUTE_SINGLE || runmode == DOCUMENT_SINGLE) {
> > +   if (uwildmat(subtest_name, single_subtest) == 0)
> > return false;
> > -   else
> > -   run_single_subtest_found = true;
> > +   else {
> > +   single_subtest_found = true;
> > +
> > +   if (runmode == DOCUMENT_SINGLE) {
> > +   if (current_subtest_documentation) {
> > +   printf("%s", 
> > current_subtest_documentation);
> > +   free(current_subtest_documentation);
> > +   current_subtest_documentation = NULL;
> > +   }
> > +
> > +   return false;
> > +   }
> > +   }
> > }
> > if (skip_subtests_henceforth) {
> > @@ -983,10 +1029,51 @@ bool __igt_run_subtest(const char *subtest_name)
> > _igt_log_buffer_reset();
> > gettime(_time);

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

2017-08-09 Thread Belgaumkar, Vinay



On 8/9/2017 4:40 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

Signed-off-by: Petri Latvala 
Acked-by: Leo Li 
---
  lib/igt_aux.c  |   8 ++--
  lib/igt_core.c | 147 +
  lib/igt_core.h |   6 ++-
  3 files changed, 126 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..f1cb0e9 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();

@@ -599,6 +608,8 @@ static void 

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

2017-08-09 Thread Arkadiusz Hiler
On Wed, Aug 09, 2017 at 02:40:49PM +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);
>   }
> 
> 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
> 
> Signed-off-by: Petri Latvala 
> Acked-by: Leo Li 

I like approach of pairing the documentation 1:1 with subtests much
better than just having comments on top of internal functions (those
does not have to map directly onto subtests).

Bonus points for having it right above the igt_subtest_f() call and
making it easily accessible from the command line as those are the two
places developers and maintainers check first.

Acked-by: Arkadiusz Hiler 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2017-08-09 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

Signed-off-by: Petri Latvala 
Acked-by: Leo Li 
---
 lib/igt_aux.c  |   8 ++--
 lib/igt_core.c | 147 +
 lib/igt_core.h |   6 ++-
 3 files changed, 126 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..f1cb0e9 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();
@@ -599,6 +608,8 @@ static void print_usage(const char *help_str, bool 
output_on_stderr)
 
fprintf(f, "Usage: %s