[v3] make abicompat test happy

Message ID 20211109122759.23613-1-tangmeng@uniontech.com
State New
Headers
Series [v3] make abicompat test happy |

Commit Message

tangmeng Nov. 9, 2021, 12:27 p.m. UTC
  When testing with runtestabicompat, the following problems were found:
1. abicompat 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.
3. The runtestabicompat command itself is executed successfully, so it
should return 0, which should be distinguished from the test result.

                * test/test-abicompat.cc (main): make test happy.

Signed-off-by: tangmeng <tangmeng@uniontech.com>
---
 tests/test-abicompat.cc | 36 +++++++++++++++++++++++++++++++-----
 tests/test-utils.h      |  4 ++++
 2 files changed, 35 insertions(+), 5 deletions(-)
  

Comments

Dodji Seketeli Nov. 9, 2021, 2:39 p.m. UTC | #1
Hello,

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

[...]

> 3. The runtestabicompat command itself is executed successfully, so it
> should return 0, which should be distinguished from the test result.

Actually, we want to return a non-zero value when any of the individual
tests fail.  This is because "make check" runs all the tests of the
regression test suite and it looks at the return value of each of the
runtestXXXX programs to detect if it failed or not.  So we really need
to be non-zero when a test case fails.

I fixed it and applied the patch below.  Thanks!

Cheers,

commit b7ba0fe8f5976251cf38f9abf249acdacdcf626b
Author: tangmeng <tangmeng@uniontech.com>
Date:   Tue Nov 9 20:27:59 2021 +0800

    test-abicompat: Make the test output more pleasant
    
    When testing with runtestabicompat, the following problems were found:
    1. abicompat 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.
    
            * test/test-abicompat.cc (main): make test output more pleasant.
    
    Signed-off-by: tangmeng <tangmeng@uniontech.com>
    Signed-off-by: Dodji Seketeli <dodji@redhat.com>

diff --git a/tests/test-abicompat.cc b/tests/test-abicompat.cc
index 49f1d5d4..a1339eef 100644
--- a/tests/test-abicompat.cc
+++ b/tests/test-abicompat.cc
@@ -27,6 +27,7 @@
 
 using std::string;
 using std::cerr;
+using std::cout;
 
 struct InOutSpec
 {
@@ -206,12 +207,13 @@ 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_app_path, in_lib1_path, in_lib2_path, suppression_path,
-    abicompat_options, ref_report_path, out_report_path, abicompat, cmd;
+    abicompat_options, ref_report_path, out_report_path, abicompat, cmd, diffcmd;
 
   for (InOutSpec* s = in_out_specs; s->in_app_path; ++s)
     {
+      bool is_ok = true;
       in_app_path = string(get_src_dir()) + "/tests/" + s->in_app_path;
       in_lib1_path = string(get_src_dir()) + "/tests/" + s->in_lib1_path;
       if (s->in_lib2_path && strcmp(s->in_lib2_path, ""))
@@ -253,13 +255,37 @@ main()
 
       if (abicompat_ok)
 	{
-	  cmd = "diff -u " + ref_report_path + " " + out_report_path;
-	  if (system(cmd.c_str()))
+	  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++;
     }
+  cout << "Summary: " << cnt_total << " tested!"
+       << " Test Passed: " << cnt_passed
+       << ", Test Failed: " << cnt_failed
+       << ".\n";
 
-  return !is_ok;
+  return cnt_failed;
 }
diff --git a/tests/test-utils.h b/tests/test-utils.h
index 46d9b785..5596edc6 100644
--- a/tests/test-utils.h
+++ b/tests/test-utils.h
@@ -9,6 +9,10 @@
 #include "config.h"
 #include <string>
 
+#define BRIGHT_YELLOW_COLOR "\e[1;33m"
+#define BRIGHT_RED_COLOR "\e[1;31m"
+#define DEFAULT_TERMINAL_COLOR "\033[0m"
+
 namespace abigail
 {
 namespace tests
  

Patch

diff --git a/tests/test-abicompat.cc b/tests/test-abicompat.cc
index 49f1d5d4..0e61b35f 100644
--- a/tests/test-abicompat.cc
+++ b/tests/test-abicompat.cc
@@ -27,6 +27,7 @@ 
 
 using std::string;
 using std::cerr;
+using std::cout;
 
 struct InOutSpec
 {
@@ -206,12 +207,13 @@  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_app_path, in_lib1_path, in_lib2_path, suppression_path,
-    abicompat_options, ref_report_path, out_report_path, abicompat, cmd;
+    abicompat_options, ref_report_path, out_report_path, abicompat, cmd, diffcmd;
 
   for (InOutSpec* s = in_out_specs; s->in_app_path; ++s)
     {
+      bool is_ok = true;
       in_app_path = string(get_src_dir()) + "/tests/" + s->in_app_path;
       in_lib1_path = string(get_src_dir()) + "/tests/" + s->in_lib1_path;
       if (s->in_lib2_path && strcmp(s->in_lib2_path, ""))
@@ -253,13 +255,37 @@  main()
 
       if (abicompat_ok)
 	{
-	  cmd = "diff -u " + ref_report_path + " " + out_report_path;
-	  if (system(cmd.c_str()))
+	  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++;
     }
+  cout << "Summary: " << cnt_total << " tested!"
+       << " Test Passed: " << cnt_passed
+       << ", Test Failed: " << cnt_failed
+       << ".\n";
 
-  return !is_ok;
+  return 0;
 }
diff --git a/tests/test-utils.h b/tests/test-utils.h
index 46d9b785..5596edc6 100644
--- a/tests/test-utils.h
+++ b/tests/test-utils.h
@@ -9,6 +9,10 @@ 
 #include "config.h"
 #include <string>
 
+#define BRIGHT_YELLOW_COLOR "\e[1;33m"
+#define BRIGHT_RED_COLOR "\e[1;31m"
+#define DEFAULT_TERMINAL_COLOR "\033[0m"
+
 namespace abigail
 {
 namespace tests