diff mbox series

[v2] make abicompat test happy

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

Commit Message

tangmeng Nov. 8, 2021, 2 a.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 | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

Comments

Dodji Seketeli Nov. 9, 2021, 11:30 a.m. UTC | #1 
Hello,

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

[...]

> +
> +      if (is_ok)
> +        {
> +          cout << "\e[1;33mTest PASSED: \033[0m" << cmd << "\n";

Hehe, bells and whistles ... :-)

I'd prefer to have the escape sequences be "self-documented" so that the
intent is clear enough.  For instance:

#define BRIGHT_YELLOW_COLOR "\a1;33m"
#define DEFAULT_TERMINAL_COLOR "\033[0m"

and then you'd you'd emit colored messages like:

cout << BRIGHT_YELLOW_COLOR
     << "Test Passed:"
     << DEFAULT_TERMINAL_COLOR
     << cmd
     << std::endl;

I believe that is more self explanatory for someone who reads the code.

Could you please use that kind of style ?

Thanks a lot for your work, by the way, it's highly appreciated!

Cheers,
tangmeng Nov. 9, 2021, 12:38 p.m. UTC | #2 
Hello， Dodji&gt;Hehe, bells and whistles ... :-) &gt; &gt;I'd prefer to have the escape sequences &gt;be "self-documented" so that the &gt;intent is clear enough.  For instance: &gt; &gt;#define BRIGHT_YELLOW_COLOR &gt;"\a1;33m" &gt;#define DEFAULT_TERMINAL_COLOR &gt;"\033[0m" &gt; &gt;and then you'd you'd emit colored &gt;messages like: &gt; &gt;cout << BRIGHT_YELLOW_COLOR &gt;     << "Test Passed:" &gt;     << DEFAULT_TERMINAL_COLOR &gt;     << cmd &gt;    << std::endl; &gt; &gt;I believe that is more self explanatory for &gt;someone who reads the code. &gt; &gt;Could you please use that kind of style ?
Thank you for your suggestions, I have reflected your suggestions in the v3 version of the patch.
&nbsp;
I put it in test-utils.h, because I also found this kind of problem in other test cases.&nbsp;
I will modify other use cases in the next time
--&nbsp;
Regards,
tangmeng
------------------&nbsp;Original&nbsp;------------------
From: &nbsp;"Dodji Seketeli"<dodji@seketeli.org&gt;;
Date: &nbsp;Tue, Nov 9, 2021 07:30 PM
To: &nbsp;"tangmeng"<tangmeng@uniontech.com&gt;; 
Cc: &nbsp;"libabigail"<libabigail@sourceware.org&gt;; 
Subject: &nbsp;Re: [PATCH v2] make abicompat test happy

&nbsp;

Hello,

tangmeng <tangmeng@uniontech.com&gt; a écrit:

[...]

&gt; +
&gt; +&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; if (is_ok)
&gt; +&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; {
&gt; +&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; cout << "\e[1;33mTest PASSED: \033[0m" << cmd << "\n";

Hehe, bells and whistles ... :-)

I'd prefer to have the escape sequences be "self-documented" so that the
intent is clear enough.&nbsp; For instance:

#define BRIGHT_YELLOW_COLOR "\a1;33m"
#define DEFAULT_TERMINAL_COLOR "\033[0m"

and then you'd you'd emit colored messages like:

cout << BRIGHT_YELLOW_COLOR
&nbsp;&nbsp;&nbsp;&nbsp; << "Test Passed:"
&nbsp;&nbsp;&nbsp;&nbsp; << DEFAULT_TERMINAL_COLOR
&nbsp;&nbsp;&nbsp;&nbsp; << cmd
&nbsp;&nbsp;&nbsp;&nbsp; << std::endl;

I believe that is more self explanatory for someone who reads the code.

Could you please use that kind of style ?

Thanks a lot for your work, by the way, it's highly appreciated!

Cheers,
diff mbox series

Patch

diff --git a/tests/test-abicompat.cc b/tests/test-abicompat.cc
index 49f1d5d4..ffcf345b 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,9 +207,10 @@  main()
   using abigail::tools_utils::ensure_parent_dir_created;
   using abigail::tools_utils::abidiff_status;
 
+  unsigned int cnt_total = 0, cnt_passed = 0, cnt_failed = 0;
   bool is_ok = true;
   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)
     {
@@ -253,13 +255,29 @@  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 << "\e[1;33mTest PASSED: \033[0m" << cmd << "\n";
+	  cnt_passed++;
+	}
+      else
+        {
+          cout << "\e[1;31mTest FAILED: \033[0m" << cmd << "\n";
+	  cnt_failed++;
+	}
+      cnt_total++;
     }
+  cout << "Summary: " << cnt_total << " tested!"
+       << " Test PASSED: " << cnt_passed
+       << ", Test FAILED: " << cnt_failed
+       << ".\n";
 
-  return !is_ok;
+  return 0;
 }