public inbox for libabigail@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Make the test output more pleasant
@ 2021-11-11  5:53 tangmeng
  2021-11-17 12:18 ` Dodji Seketeli
  0 siblings, 1 reply; 2+ messages in thread
From: tangmeng @ 2021-11-11  5:53 UTC (permalink / raw)
  To: libabigail; +Cc: tangmeng

Sync with the commit id of b7ba0fe8f5976251cf38f9abf249acdacdcf626b.
When testing, the following problems were found:
1. command tested multiple scenarios, but the last result was used
as the basis for the return value of the command.
2. For multiple test scenarios, the execution results cannot be known
after the test, which is easy to cause confusion.

        * tests/test-abidiff-exit.cc (main): make test output more pleasant.
        * tests/test-alt-dwarf-file.cc (main): make test output more pleasant.
        * tests/test-annotate.cc (main): make test output more pleasant.
        * tests/test-diff-dwarf-abixml.cc (main): make test output more pleasant.
        * tests/test-ini.cc (main): make test output more pleasant.
        * tests/test-lookup-syms.cc (main): make test output more pleasant.

Signed-off-by: tangmeng <tangmeng@uniontech.com>
---
 tests/test-abidiff-exit.cc      | 38 ++++++++++++++++++++++++++++-----
 tests/test-alt-dwarf-file.cc    | 35 ++++++++++++++++++++++++++----
 tests/test-annotate.cc          | 37 +++++++++++++++++++++++++++-----
 tests/test-diff-dwarf-abixml.cc | 38 ++++++++++++++++++++++++++++-----
 tests/test-ini.cc               | 38 ++++++++++++++++++++++++++++-----
 tests/test-lookup-syms.cc       | 36 +++++++++++++++++++++++++++----
 6 files changed, 194 insertions(+), 28 deletions(-)

diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
index 9ed1d6d7..e9af39e5 100644
--- a/tests/test-abidiff-exit.cc
+++ b/tests/test-abidiff-exit.cc
@@ -27,6 +27,7 @@
 #include "test-utils.h"
 
 using abigail::tools_utils::abidiff_status;
+using std::cout;
 
 struct InOutSpec
 {
@@ -453,9 +454,10 @@ main()
   using abigail::tools_utils::split_string;
   using abigail::tools_utils::abidiff_status;
 
-  bool is_ok = true;
+  unsigned int cnt_total = 0, cnt_passed = 0, cnt_failed = 0;
+
   string in_elfv0_path, in_elfv1_path,
-    in_suppression_path, abidiff_options, abidiff, cmd,
+    in_suppression_path, abidiff_options, abidiff, cmd, diffcmd,
     ref_diff_report_path, out_diff_report_path;
   vector<string> in_elfv0_headers_dirs, in_elfv1_headers_dirs;
   string source_dir_prefix = string(get_src_dir()) + "/tests/";
@@ -463,6 +465,7 @@ main()
 
     for (InOutSpec* s = in_out_specs; s->in_elfv0_path; ++s)
       {
+        bool is_ok = true;
 	in_elfv0_path = source_dir_prefix + s->in_elfv0_path;
 	in_elfv1_path = source_dir_prefix + s->in_elfv1_path;
 	split_string(s->in_elfv0_headers_dirs, ",", in_elfv0_headers_dirs);
@@ -529,14 +532,39 @@ main()
 
 	if (abidiff_ok)
 	  {
-	    cmd = "diff -u " + ref_diff_report_path
+	    diffcmd = "diff -u " + ref_diff_report_path
 	      + " " + out_diff_report_path;
-	    if (system(cmd.c_str()))
+	    if (system(diffcmd.c_str()))
 	      is_ok = false;
 	  }
 	else
 	  is_ok = false;
+
+        if (is_ok)
+          {
+            cout << BRIGHT_YELLOW_COLOR
+                 << "Test Passed: "
+                 << DEFAULT_TERMINAL_COLOR
+                 << cmd
+                 << std::endl;
+            cnt_passed++;
+          }
+        else
+          {
+            cout << BRIGHT_RED_COLOR
+                 << "Test Failed: "
+                 << DEFAULT_TERMINAL_COLOR
+                 << cmd
+                 << std::endl;
+            cnt_failed++;
+          }
+        cnt_total++;
       }
 
-    return !is_ok;
+    cout << "Summary: " << cnt_total << " tested!"
+         << " Test Passed: " << cnt_passed
+         << ", Test Failed: " << cnt_failed
+         << ".\n";
+
+    return cnt_failed;
 }
diff --git a/tests/test-alt-dwarf-file.cc b/tests/test-alt-dwarf-file.cc
index b774a1f6..c1f2fb38 100644
--- a/tests/test-alt-dwarf-file.cc
+++ b/tests/test-alt-dwarf-file.cc
@@ -16,6 +16,7 @@
 #include "test-utils.h"
 
 using std::cerr;
+using std::cout;
 using std::string;
 
 struct InOutSpec
@@ -70,6 +71,8 @@ main()
   using abigail::tests::get_build_dir;
   using abigail::tools_utils::ensure_parent_dir_created;
 
+  unsigned int cnt_total = 0, cnt_passed = 0, cnt_failed = 0;
+
   bool is_ok = true;
   string in_elf_path, ref_report_path, out_report_path, debug_info_dir;
   string abidw, abidw_options;
@@ -101,16 +104,40 @@ main()
 
       if (abidw_ok)
 	{
-	  cmd = "diff -u " + ref_report_path + " " + out_report_path;
-	  if (system(cmd.c_str()))
+	  string diffcmd = "diff -u " + ref_report_path + " " + out_report_path;
+	  if (system(diffcmd.c_str()))
 	    is_ok &=false;
 	}
       else
 	{
-	  cerr << "command failed: " << cmd << "\n";
 	  is_ok &= false;
 	}
+
+      if (is_ok)
+        {
+          cout << BRIGHT_YELLOW_COLOR
+               << "Test Passed: "
+               << DEFAULT_TERMINAL_COLOR
+               << cmd
+               << std::endl;
+          cnt_passed++;
+        }
+      else
+        {
+          cout << BRIGHT_RED_COLOR
+               << "Test Failed: "
+               << DEFAULT_TERMINAL_COLOR
+               << cmd
+               << std::endl;
+          cnt_failed++;
+        }
+      cnt_total++;
     }
 
-  return !is_ok;
+  cout << "Summary: " << cnt_total << " tested!"
+       << " Test Passed: " << cnt_passed
+       << ", Test Failed: " << cnt_failed
+       << ".\n";
+
+  return cnt_failed;
 }
diff --git a/tests/test-annotate.cc b/tests/test-annotate.cc
index d6504dd4..23a87799 100644
--- a/tests/test-annotate.cc
+++ b/tests/test-annotate.cc
@@ -13,6 +13,7 @@
 #include "test-utils.h"
 
 using std::cerr;
+using std::cout;
 using std::string;
 
 struct InOutSpec
@@ -140,7 +141,8 @@ main()
   using abigail::tests::get_build_dir;
   using abigail::tools_utils::ensure_parent_dir_created;
 
-  bool is_ok = true;
+  unsigned int cnt_total = 0, cnt_passed = 0, cnt_failed = 0;
+
   string in_elf_path, ref_report_path, out_report_path;
   string abidw;
 
@@ -148,6 +150,7 @@ main()
     "--annotate --no-corpus-path";
   for (InOutSpec* s = in_out_specs; s->in_elf_path; ++s)
     {
+      bool is_ok = true;
       in_elf_path = string(get_src_dir()) + "/tests/" + s->in_elf_path;
       ref_report_path = string(get_src_dir()) + "/tests/" + s->in_report_path;
       out_report_path =
@@ -168,16 +171,40 @@ main()
 
       if (abidw_ok)
 	{
-	  cmd = "diff -u " + ref_report_path + " " + out_report_path;
-	  if (system(cmd.c_str()))
+	  string diffcmd = "diff -u " + ref_report_path + " " + out_report_path;
+	  if (system(diffcmd.c_str()))
 	    is_ok &=false;
 	}
       else
 	{
-	  cerr << "command failed: " << cmd << "\n";
 	  is_ok &= false;
 	}
+
+      if (is_ok)
+        {
+          cout << BRIGHT_YELLOW_COLOR
+               << "Test Passed: "
+               << DEFAULT_TERMINAL_COLOR
+               << cmd
+               << std::endl;
+          cnt_passed++;
+        }
+      else
+        {
+          cout << BRIGHT_RED_COLOR
+               << "Test Failed: "
+               << DEFAULT_TERMINAL_COLOR
+               << cmd
+               << std::endl;
+          cnt_failed++;
+        }
+      cnt_total++;
     }
 
-  return !is_ok;
+  cout << "Summary: " << cnt_total << " tested!"
+       << " Test Passed: " << cnt_passed
+       << ", Test Failed: " << cnt_failed
+       << ".\n";
+
+  return cnt_failed;
 }
diff --git a/tests/test-diff-dwarf-abixml.cc b/tests/test-diff-dwarf-abixml.cc
index ba85415e..3bd1ce29 100644
--- a/tests/test-diff-dwarf-abixml.cc
+++ b/tests/test-diff-dwarf-abixml.cc
@@ -19,6 +19,7 @@
 using std::string;
 using std::ofstream;
 using std::cerr;
+using std::cout;
 using abigail::tests::get_build_dir;
 
 /// Specifies where a test should get its inputs from, and where it
@@ -57,12 +58,14 @@ main()
   using abigail::tools_utils::ensure_parent_dir_created;
   using abigail::tools_utils::abidiff_status;
 
-  bool is_ok = true;
+  unsigned int cnt_total = 0, cnt_passed = 0, cnt_failed = 0;
+
   string in_elf_path, in_abi_path,
-    abidiff, cmd, ref_diff_report_path, out_diff_report_path;
+    abidiff, cmd, diffcmd, ref_diff_report_path, out_diff_report_path;
 
   for (InOutSpec* s = in_out_specs; s->in_elf_path; ++s)
     {
+      bool is_ok = true;
       in_elf_path = string(get_src_dir()) + "/tests/" + s->in_elf_path;
       in_abi_path = string(get_src_dir()) + "/tests/"+ s->in_abi_path;
       ref_diff_report_path =
@@ -96,14 +99,39 @@ main()
 	}
       if (abidiff_ok)
 	{
-	  cmd = "diff -u " + ref_diff_report_path
+	  diffcmd = "diff -u " + ref_diff_report_path
 	    + " " + out_diff_report_path;
-	  if (system(cmd.c_str()))
+	  if (system(diffcmd.c_str()))
 	    is_ok = false;
 	}
       else
 	is_ok = false;
+
+      if (is_ok)
+        {
+          cout << BRIGHT_YELLOW_COLOR
+               << "Test Passed: "
+               << DEFAULT_TERMINAL_COLOR
+               << cmd
+               << std::endl;
+          cnt_passed++;
+        }
+      else
+        {
+          cout << BRIGHT_RED_COLOR
+               << "Test Failed: "
+               << DEFAULT_TERMINAL_COLOR
+               << cmd
+               << std::endl;
+          cnt_failed++;
+        }
+      cnt_total++;
     }
 
-  return !is_ok;
+  cout << "Summary: " << cnt_total << " tested!"
+       << " Test Passed: " << cnt_passed
+       << ", Test Failed: " << cnt_failed
+       << ".\n";
+
+  return cnt_failed;
 }
diff --git a/tests/test-ini.cc b/tests/test-ini.cc
index f906e50b..0e1c850e 100644
--- a/tests/test-ini.cc
+++ b/tests/test-ini.cc
@@ -21,6 +21,7 @@
 
 using std::string;
 using std::cerr;
+using std::cout;
 
 /// This is an aggregate that specifies where a test shall get its
 /// input from and where it shall write its ouput to.
@@ -82,11 +83,13 @@ main()
   using abigail::tools_utils::ensure_parent_dir_created;
   using abigail::tools_utils::abidiff_status;
 
-  bool is_ok = true;
-  string in_ini_path, in_reference_output_path, out_ini_path, cmd;
+  unsigned int cnt_total = 0, cnt_passed = 0, cnt_failed = 0;
+
+  string in_ini_path, in_reference_output_path, out_ini_path, cmd, diffcmd;
 
   for (InOutSpec *s = in_out_specs; s->in_ini_path; ++s)
     {
+      bool is_ok = true;
       in_ini_path = get_test_src_dir() + "/" + s->in_ini_path;
       in_reference_output_path =
 	get_test_src_dir() + "/" + s->in_reference_output_path;
@@ -120,13 +123,38 @@ main()
 
       if (cmd_is_ok)
 	{
-	  cmd = "diff -u " + in_reference_output_path + " " + out_ini_path;
-	  if (system(cmd.c_str()))
+	  diffcmd = "diff -u " + in_reference_output_path + " " + out_ini_path;
+	  if (system(diffcmd.c_str()))
 	    is_ok = false;
 	}
       else
 	is_ok = false;
+
+      if (is_ok)
+        {
+          cout << BRIGHT_YELLOW_COLOR
+               << "Test Passed: "
+               << DEFAULT_TERMINAL_COLOR
+               << cmd
+               << std::endl;
+          cnt_passed++;
+        }
+      else
+        {
+          cout << BRIGHT_RED_COLOR
+               << "Test Failed: "
+               << DEFAULT_TERMINAL_COLOR
+               << cmd
+               << std::endl;
+          cnt_failed++;
+        }
+      cnt_total++;
     }
 
-  return !is_ok;
+  cout << "Summary: " << cnt_total << " tested!"
+       << " Test Passed: " << cnt_passed
+       << ", Test Failed: " << cnt_failed
+       << ".\n";
+
+  return cnt_failed;
 }
diff --git a/tests/test-lookup-syms.cc b/tests/test-lookup-syms.cc
index 5c63868e..c3a55fb4 100644
--- a/tests/test-lookup-syms.cc
+++ b/tests/test-lookup-syms.cc
@@ -17,6 +17,7 @@
 #include "test-utils.h"
 
 using std::cerr;
+using std::cout;
 using std::string;
 
 struct InOutSpec
@@ -90,12 +91,14 @@ main()
   using abigail::tests::get_build_dir;
   using abigail::tools_utils::ensure_parent_dir_created;
 
-  bool is_ok = true;
+  unsigned int cnt_total = 0, cnt_passed = 0, cnt_failed = 0;
+
   string in_elf_path, symbol, abisym, abisym_options,
     ref_report_path, out_report_path;
 
   for (InOutSpec* s = in_out_specs; s->in_elf_path; ++s)
     {
+      bool is_ok = true;
       in_elf_path = string(get_src_dir()) + "/tests/" + s->in_elf_path;
       symbol = s->symbol;
       abisym_options = s->abisym_options;
@@ -123,13 +126,38 @@ main()
 
       if (abisym_ok)
 	{
-	  cmd = "diff -u " + ref_report_path + " "+  out_report_path;
-	  if (system(cmd.c_str()))
+	  string diffcmd = "diff -u " + ref_report_path + " "+  out_report_path;
+	  if (system(diffcmd.c_str()))
 	    is_ok = false;
 	}
       else
 	is_ok = false;
+
+      if (is_ok)
+        {
+          cout << BRIGHT_YELLOW_COLOR
+               << "Test Passed: "
+               << DEFAULT_TERMINAL_COLOR
+               << cmd
+               << std::endl;
+          cnt_passed++;
+        }
+      else
+        {
+          cout << BRIGHT_RED_COLOR
+               << "Test Failed: "
+               << DEFAULT_TERMINAL_COLOR
+               << cmd
+               << std::endl;
+          cnt_failed++;
+        }
+      cnt_total++;
     }
 
-  return !is_ok;
+  cout << "Summary: " << cnt_total << " tested!"
+       << " Test Passed: " << cnt_passed
+       << ", Test Failed: " << cnt_failed
+       << ".\n";
+
+  return cnt_failed;
 }
-- 
2.20.1




^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] Make the test output more pleasant
  2021-11-11  5:53 [PATCH] Make the test output more pleasant tangmeng
@ 2021-11-17 12:18 ` Dodji Seketeli
  0 siblings, 0 replies; 2+ messages in thread
From: Dodji Seketeli @ 2021-11-17 12:18 UTC (permalink / raw)
  To: tangmeng; +Cc: libabigail

Hello,

tangmeng <tangmeng@uniontech.com> a écrit:

Thank you for the patch!

I see that you have a real interest in ameliorating the output of the
tests.  Thank you for that!

As I am seeing more of these patches coming from you, I looked at them a
little bit deeper and came up with several observations.  I went ahead
and made various changes that complement yours.

Please find my comments below.

[...]


>         * tests/test-abidiff-exit.cc (main): make test output more pleasant.
>         * tests/test-alt-dwarf-file.cc (main): make test output more pleasant.
>         * tests/test-annotate.cc (main): make test output more pleasant.
>         * tests/test-diff-dwarf-abixml.cc (main): make test output more pleasant.
>         * tests/test-ini.cc (main): make test output more pleasant.
>         * tests/test-lookup-syms.cc (main): make test output more pleasant.

Here, the space before the '*' must be a <tab>, as explained in the
COMMIT-LOG-GUIDELINES file that is in the source tree.  You can browse
it at
https://sourceware.org/git/?p=libabigail.git;a=blob_plain;f=COMMIT-LOG-GUIDELINES;hb=HEAD.


> diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc

[...]


> +        if (is_ok)
> +          {
> +            cout << BRIGHT_YELLOW_COLOR
> +                 << "Test Passed: "
> +                 << DEFAULT_TERMINAL_COLOR
> +                 << cmd
> +                 << std::endl;

Emitting the command line of every single test is way too verbose in my
opinion.  By default, we want to have minimum information on the screen.
When something goes wrong however, we want more details so that we can
debug/investigate the issue ...

[...]

> +        else
> +          {
> +            cout << BRIGHT_RED_COLOR
> +                 << "Test Failed: "
> +                 << DEFAULT_TERMINAL_COLOR
> +                 << cmd
> +                 << std::endl;
> +            cnt_failed++;

... so here, it's cool to have the command line of the test that is
failing so that we can just run it in the debugger and investigate what
is going wrong.

> +          }
> +        cnt_total++;
>        }

This pattern has been repeated over and over in several tests.  I have
factorized it out into a function named
abigail::tests::emit_test_status_and_update_counters.  This function
does not emit the command line of the tests that succeed.  Rather, it
emits the command line of the test that fails.

You'll see below the patch where I define that function.

[...]

> +    cout << "Summary: " << cnt_total << " tested!"
> +         << " Test Passed: " << cnt_passed
> +         << ", Test Failed: " << cnt_failed
> +         << ".\n";

This pattern has also been repeated over and over in several tests of
this patch so I have factorized it out into a function named
abigail::tests::emit_test_summary.

You'll see below the patch where I define that function.

[...]

> diff --git a/tests/test-alt-dwarf-file.cc b/tests/test-alt-dwarf-file.cc

[...]

> diff --git a/tests/test-annotate.cc b/tests/test-annotate.cc

[...]

> diff --git a/tests/test-diff-dwarf-abixml.cc b/tests/test-diff-dwarf-abixml.cc

[...]

> diff --git a/tests/test-ini.cc b/tests/test-ini.cc

[...]

> diff --git a/tests/test-lookup-syms.cc b/tests/test-lookup-syms.cc

[...]

I have update all these files to make them use the two new functions I
have defined.

Please find below the three patches that I have applied to master,
complementing your work.  Your patch is the last one.  I have amended it
to make it use the first two patches and re-worded some comments and
variable names.

I guess future changes to test output should be based on this framework
being put in place now.

Thanks again for your work!

From c9e74e49d68eecf004be7f52277b90a4ddcf764d Mon Sep 17 00:00:00 2001
From: Dodji Seketeli <dodji@redhat.com>
Date: Wed, 17 Nov 2021 11:58:31 +0100
Subject: [PATCH 1/3] test-utils: Define colors for test status messages

This patch defines pre-processor macros for the colors used to emit
test SUCCESS/FAILURE status.  These are going to be used by the code,
onward.

	* tests/test-utils.h (TEST_FAILURE_COLOR, TEST_SUCCESS_COLOR):
	Define macros.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
---
 tests/test-utils.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/test-utils.h b/tests/test-utils.h
index 5596edc6..9f6d1e06 100644
--- a/tests/test-utils.h
+++ b/tests/test-utils.h
@@ -13,6 +13,9 @@
 #define BRIGHT_RED_COLOR "\e[1;31m"
 #define DEFAULT_TERMINAL_COLOR "\033[0m"
 
+#define TEST_FAILURE_COLOR BRIGHT_RED_COLOR
+#define TEST_SUCCESS_COLOR BRIGHT_YELLOW_COLOR
+
 namespace abigail
 {
 namespace tests
-- 
2.32.0


From 41625582a3bb5a4303b60aef4e3632e7f581dce9 Mon Sep 17 00:00:00 2001
From: Dodji Seketeli <dodji@redhat.com>
Date: Wed, 17 Nov 2021 12:00:52 +0100
Subject: [PATCH 2/3] test-utils: Define test status reporting functions

	* tests/test-utils.h (emit_test_status_and_update_counters)
	(emit_test_summary): Declare ...
	* tests/test-utils.cc (emit_test_status_and_update_counters)
	(emit_test_summary): ... new functions.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
---
 tests/test-utils.cc | 63 +++++++++++++++++++++++++++++++++++++++++++++
 tests/test-utils.h  | 11 +++++++-
 2 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/tests/test-utils.cc b/tests/test-utils.cc
index db169cc3..f317c580 100644
--- a/tests/test-utils.cc
+++ b/tests/test-utils.cc
@@ -3,6 +3,7 @@
 //
 // Copyright (C) 2013-2020 Red Hat, Inc.
 
+#include <iostream>
 #include "test-utils.h"
 
 using std::string;
@@ -40,5 +41,67 @@ get_build_dir()
   return s;
 }
 
+/// Emit test status on the standard output.
+///
+/// This function also increments passed, failed and total test
+/// numbers accordingly.
+///
+/// @param test_passed indicated if the test succeeded or not.
+///
+/// @param test_cmd the test command that was executed.  If the test
+/// failed, the exact command is displayed.
+///
+/// @param passed_count the number of passed tests.  This is going to
+/// be incremented if the test passes.
+///
+/// @param failed_count the number of failed tests.  This is going to
+/// be incremented if the test fails.
+///
+/// @param total_count the total number of tests.  This is going to be
+/// incremented.
+void
+emit_test_status_and_update_counters(bool test_passed,
+				     const std::string& test_cmd,
+				     unsigned& passed_count,
+				     unsigned& failed_count,
+				     unsigned& total_count)
+{
+  if (test_passed)
+    passed_count++;
+  else
+    {
+      std::cout << TEST_FAILURE_COLOR
+		<< "Test Failed: "
+		<< DEFAULT_TERMINAL_COLOR
+		<< test_cmd
+		<< std::endl;
+      failed_count++;
+    }
+  total_count++;
+}
+
+/// Emit the summary of the test.
+///
+/// @param total_count the total number of tests executed.
+///
+/// @param passed_count the number of tests that succeeded.
+///
+/// @param failed_count the number of tests that failed.
+void
+emit_test_summary(unsigned total_count,
+		  unsigned passed_count,
+		  unsigned failed_count)
+{
+  if (failed_count)
+    std::cout << TEST_FAILURE_COLOR << "FAILURE!";
+  else
+    std::cout << TEST_SUCCESS_COLOR << "SUCCESS!";
+  std::cout << DEFAULT_TERMINAL_COLOR << "\n";
+
+  std::cout << "Total number of tests executed: " << total_count
+	    << " Number of tests PASSED: " << passed_count
+	    << ", Number of tests FAILED: " << failed_count
+	    << ".\n";
+}
 }//end namespace tests
 }//end namespace abigail
diff --git a/tests/test-utils.h b/tests/test-utils.h
index 9f6d1e06..e1fd17d5 100644
--- a/tests/test-utils.h
+++ b/tests/test-utils.h
@@ -23,7 +23,16 @@ namespace tests
 
 const char* get_src_dir();
 const char* get_build_dir();
-
+void
+emit_test_status_and_update_counters(bool test_passed,
+				     const std::string& test_cmd,
+				     unsigned& passed_count,
+				     unsigned& failed_count,
+				     unsigned& total_count);
+void
+emit_test_summary(unsigned total_count,
+		  unsigned passed_count,
+		  unsigned failed_count);
 }//end namespace tests
 }//end namespace abigail
 #endif //__TEST_UTILS_H__
-- 
2.32.0


From eea4e92b2d106e1d845738d8f5fdaf12ed0e4c81 Mon Sep 17 00:00:00 2001
From: tangmeng <tangmeng@uniontech.com>
Date: Thu, 11 Nov 2021 13:53:42 +0800
Subject: [PATCH 3/3] Standardize and improve the output of several tests

This patch updates several test harnesses to make their output show
the command line of the failing tests, a brief informative summary
about the number of unit tests executed, failed and executed with
success.

These tests now used the
abigail::tests::emit_test_{summary,status_and_update_counters}
functions provided in tests/test-utils.cc.

	* tests/test-abidiff-exit.cc (main): Use
	abigail::tests::emit_test_{summary, status_and_update_counters}
	functions to ameliorate and standardize test output.
	* tests/test-alt-dwarf-file.cc (main): Likewise.
	* tests/test-annotate.cc (main): Likewise.
	* tests/test-diff-dwarf-abixml.cc (main): Likewise.
	* tests/test-ini.cc (main): Likewise.
	* tests/test-lookup-syms.cc (main): Likewise.

Signed-off-by: tangmeng <tangmeng@uniontech.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
---
 tests/test-abidiff-exit.cc      | 23 ++++++++++++++++++-----
 tests/test-alt-dwarf-file.cc    | 19 ++++++++++++-------
 tests/test-annotate.cc          | 19 ++++++++++++++-----
 tests/test-diff-dwarf-abixml.cc | 19 ++++++++++++++-----
 tests/test-ini.cc               | 19 ++++++++++++++-----
 tests/test-lookup-syms.cc       | 17 +++++++++++++----
 6 files changed, 85 insertions(+), 31 deletions(-)

diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
index 9ed1d6d7..15acfed9 100644
--- a/tests/test-abidiff-exit.cc
+++ b/tests/test-abidiff-exit.cc
@@ -27,6 +27,8 @@
 #include "test-utils.h"
 
 using abigail::tools_utils::abidiff_status;
+using abigail::tests::emit_test_status_and_update_counters;
+using abigail::tests::emit_test_summary;
 
 struct InOutSpec
 {
@@ -453,9 +455,10 @@ main()
   using abigail::tools_utils::split_string;
   using abigail::tools_utils::abidiff_status;
 
-  bool is_ok = true;
+  unsigned int total_count = 0, passed_count = 0, failed_count = 0;
+
   string in_elfv0_path, in_elfv1_path,
-    in_suppression_path, abidiff_options, abidiff, cmd,
+    in_suppression_path, abidiff_options, abidiff, cmd, diff_cmd,
     ref_diff_report_path, out_diff_report_path;
   vector<string> in_elfv0_headers_dirs, in_elfv1_headers_dirs;
   string source_dir_prefix = string(get_src_dir()) + "/tests/";
@@ -463,6 +466,7 @@ main()
 
     for (InOutSpec* s = in_out_specs; s->in_elfv0_path; ++s)
       {
+	bool is_ok = true;
 	in_elfv0_path = source_dir_prefix + s->in_elfv0_path;
 	in_elfv1_path = source_dir_prefix + s->in_elfv1_path;
 	split_string(s->in_elfv0_headers_dirs, ",", in_elfv0_headers_dirs);
@@ -529,14 +533,23 @@ main()
 
 	if (abidiff_ok)
 	  {
-	    cmd = "diff -u " + ref_diff_report_path
+	    diff_cmd = "diff -u " + ref_diff_report_path
 	      + " " + out_diff_report_path;
-	    if (system(cmd.c_str()))
+	    if (system(diff_cmd.c_str()))
 	      is_ok = false;
 	  }
 	else
 	  is_ok = false;
+
+	emit_test_status_and_update_counters(is_ok,
+					     cmd,
+					     passed_count,
+					     failed_count,
+					     total_count);
       }
 
-    return !is_ok;
+    emit_test_summary(total_count, passed_count, failed_count);
+
+
+    return failed_count;
 }
diff --git a/tests/test-alt-dwarf-file.cc b/tests/test-alt-dwarf-file.cc
index b774a1f6..315296ab 100644
--- a/tests/test-alt-dwarf-file.cc
+++ b/tests/test-alt-dwarf-file.cc
@@ -17,6 +17,8 @@
 
 using std::cerr;
 using std::string;
+using abigail::tests::emit_test_status_and_update_counters;
+using abigail::tests::emit_test_summary;
 
 struct InOutSpec
 {
@@ -70,6 +72,8 @@ main()
   using abigail::tests::get_build_dir;
   using abigail::tools_utils::ensure_parent_dir_created;
 
+  unsigned int total_count = 0, passed_count = 0, failed_count = 0;
+
   bool is_ok = true;
   string in_elf_path, ref_report_path, out_report_path, debug_info_dir;
   string abidw, abidw_options;
@@ -101,16 +105,17 @@ main()
 
       if (abidw_ok)
 	{
-	  cmd = "diff -u " + ref_report_path + " " + out_report_path;
-	  if (system(cmd.c_str()))
+	  string diff_cmd = "diff -u " + ref_report_path + " " + out_report_path;
+	  if (system(diff_cmd.c_str()))
 	    is_ok &=false;
 	}
       else
-	{
-	  cerr << "command failed: " << cmd << "\n";
-	  is_ok &= false;
-	}
+	is_ok &= false;
+
+      emit_test_status_and_update_counters(is_ok, cmd, passed_count,
+					   failed_count, total_count);
     }
 
-  return !is_ok;
+  emit_test_summary(total_count, passed_count, failed_count);
+  return failed_count;
 }
diff --git a/tests/test-annotate.cc b/tests/test-annotate.cc
index d6504dd4..bc4ce852 100644
--- a/tests/test-annotate.cc
+++ b/tests/test-annotate.cc
@@ -14,6 +14,8 @@
 
 using std::cerr;
 using std::string;
+using abigail::tests::emit_test_status_and_update_counters;
+using abigail::tests::emit_test_summary;
 
 struct InOutSpec
 {
@@ -140,7 +142,8 @@ main()
   using abigail::tests::get_build_dir;
   using abigail::tools_utils::ensure_parent_dir_created;
 
-  bool is_ok = true;
+  unsigned int total_count = 0, passed_count = 0, failed_count = 0;
+
   string in_elf_path, ref_report_path, out_report_path;
   string abidw;
 
@@ -148,6 +151,7 @@ main()
     "--annotate --no-corpus-path";
   for (InOutSpec* s = in_out_specs; s->in_elf_path; ++s)
     {
+      bool is_ok = true;
       in_elf_path = string(get_src_dir()) + "/tests/" + s->in_elf_path;
       ref_report_path = string(get_src_dir()) + "/tests/" + s->in_report_path;
       out_report_path =
@@ -168,16 +172,21 @@ main()
 
       if (abidw_ok)
 	{
-	  cmd = "diff -u " + ref_report_path + " " + out_report_path;
-	  if (system(cmd.c_str()))
+	  string diff_cmd =
+	    "diff -u " + ref_report_path + " " + out_report_path;
+	  if (system(diff_cmd.c_str()))
 	    is_ok &=false;
 	}
       else
 	{
-	  cerr << "command failed: " << cmd << "\n";
 	  is_ok &= false;
 	}
+
+      emit_test_status_and_update_counters(is_ok, cmd, passed_count,
+					   failed_count, total_count);
     }
 
-  return !is_ok;
+  emit_test_summary(total_count, passed_count, failed_count);
+
+  return failed_count;
 }
diff --git a/tests/test-diff-dwarf-abixml.cc b/tests/test-diff-dwarf-abixml.cc
index 66ba81fa..deb7b25a 100644
--- a/tests/test-diff-dwarf-abixml.cc
+++ b/tests/test-diff-dwarf-abixml.cc
@@ -20,6 +20,8 @@ using std::string;
 using std::ofstream;
 using std::cerr;
 using abigail::tests::get_build_dir;
+using abigail::tests::emit_test_status_and_update_counters;
+using abigail::tests::emit_test_summary;
 
 /// Specifies where a test should get its inputs from, and where it
 /// should write its output to.
@@ -57,12 +59,14 @@ main()
   using abigail::tools_utils::ensure_parent_dir_created;
   using abigail::tools_utils::abidiff_status;
 
-  bool is_ok = true;
+  unsigned int total_count = 0, passed_count = 0, failed_count = 0;
+
   string in_elf_path, in_abi_path,
-    abidiff, cmd, ref_diff_report_path, out_diff_report_path;
+    abidiff, cmd, diff_cmd, ref_diff_report_path, out_diff_report_path;
 
   for (InOutSpec* s = in_out_specs; s->in_elf_path; ++s)
     {
+      bool is_ok = true;
       in_elf_path = string(get_src_dir()) + "/tests/" + s->in_elf_path;
       in_abi_path = string(get_src_dir()) + "/tests/"+ s->in_abi_path;
       ref_diff_report_path =
@@ -102,14 +106,19 @@ main()
 	}
       if (abidiff_ok)
 	{
-	  cmd = "diff -u " + ref_diff_report_path
+	  diff_cmd = "diff -u " + ref_diff_report_path
 	    + " " + out_diff_report_path;
-	  if (system(cmd.c_str()))
+	  if (system(diff_cmd.c_str()))
 	    is_ok = false;
 	}
       else
 	is_ok = false;
+
+      emit_test_status_and_update_counters(is_ok, cmd, passed_count,
+					   failed_count, total_count);
     }
 
-  return !is_ok;
+  emit_test_summary(total_count, passed_count, failed_count);
+
+  return failed_count;
 }
diff --git a/tests/test-ini.cc b/tests/test-ini.cc
index f906e50b..5d41efbc 100644
--- a/tests/test-ini.cc
+++ b/tests/test-ini.cc
@@ -21,6 +21,8 @@
 
 using std::string;
 using std::cerr;
+using abigail::tests::emit_test_status_and_update_counters;
+using abigail::tests::emit_test_summary;
 
 /// This is an aggregate that specifies where a test shall get its
 /// input from and where it shall write its ouput to.
@@ -82,11 +84,13 @@ main()
   using abigail::tools_utils::ensure_parent_dir_created;
   using abigail::tools_utils::abidiff_status;
 
-  bool is_ok = true;
-  string in_ini_path, in_reference_output_path, out_ini_path, cmd;
+  unsigned int total_count = 0, passed_count = 0, failed_count = 0;
+
+  string in_ini_path, in_reference_output_path, out_ini_path, cmd, diff_cmd;
 
   for (InOutSpec *s = in_out_specs; s->in_ini_path; ++s)
     {
+      bool is_ok = true;
       in_ini_path = get_test_src_dir() + "/" + s->in_ini_path;
       in_reference_output_path =
 	get_test_src_dir() + "/" + s->in_reference_output_path;
@@ -120,13 +124,18 @@ main()
 
       if (cmd_is_ok)
 	{
-	  cmd = "diff -u " + in_reference_output_path + " " + out_ini_path;
-	  if (system(cmd.c_str()))
+	  diff_cmd = "diff -u " + in_reference_output_path + " " + out_ini_path;
+	  if (system(diff_cmd.c_str()))
 	    is_ok = false;
 	}
       else
 	is_ok = false;
+
+      emit_test_status_and_update_counters(is_ok, cmd, passed_count,
+					   failed_count, total_count);
     }
 
-  return !is_ok;
+  emit_test_summary(total_count, passed_count, failed_count);
+
+  return failed_count;
 }
diff --git a/tests/test-lookup-syms.cc b/tests/test-lookup-syms.cc
index 5c63868e..096a303e 100644
--- a/tests/test-lookup-syms.cc
+++ b/tests/test-lookup-syms.cc
@@ -18,6 +18,8 @@
 
 using std::cerr;
 using std::string;
+using abigail::tests::emit_test_status_and_update_counters;
+using abigail::tests::emit_test_summary;
 
 struct InOutSpec
 {
@@ -90,12 +92,14 @@ main()
   using abigail::tests::get_build_dir;
   using abigail::tools_utils::ensure_parent_dir_created;
 
-  bool is_ok = true;
+  unsigned int total_count = 0, passed_count = 0, failed_count = 0;
+
   string in_elf_path, symbol, abisym, abisym_options,
     ref_report_path, out_report_path;
 
   for (InOutSpec* s = in_out_specs; s->in_elf_path; ++s)
     {
+      bool is_ok = true;
       in_elf_path = string(get_src_dir()) + "/tests/" + s->in_elf_path;
       symbol = s->symbol;
       abisym_options = s->abisym_options;
@@ -123,13 +127,18 @@ main()
 
       if (abisym_ok)
 	{
-	  cmd = "diff -u " + ref_report_path + " "+  out_report_path;
-	  if (system(cmd.c_str()))
+	  string diff_cmd = "diff -u " + ref_report_path + " "+  out_report_path;
+	  if (system(diff_cmd.c_str()))
 	    is_ok = false;
 	}
       else
 	is_ok = false;
+
+      emit_test_status_and_update_counters(is_ok, cmd, passed_count,
+					   failed_count, total_count);
     }
 
-  return !is_ok;
+  emit_test_summary(total_count, passed_count, failed_count);
+
+  return failed_count;
 }
-- 
2.32.0


-- 
		Dodji

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-11-17 12:18 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-11  5:53 [PATCH] Make the test output more pleasant tangmeng
2021-11-17 12:18 ` Dodji Seketeli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).