Patch for unit test failure and error only on Solaris

[Jim Li-3]

Hi All,

This patch fix two different behavior between Linux and Solaris:
1. The default shell of system on Solaris is sh which can't explain
"test \"x$(ls data1)\" = \"xtestdata\"" correctly. we use `` to replace
$ which can work well both for sh and bash which means it make unit test
work well both on Linux and on Solaris
2. Solaris's diff doesn't support "-x " options, and it has gnu diff
which name is gdiff. so we use a micro definition to fix this problem.

Almost all unit test has this kind of problem. this patch is just a
sample  patch only for check_filter. if you guys think it's fine, I'll
deal with all of unit test for this issue, so many edit work ...

welcome any suggestion and comments!

Thanks

Jim


diff -ru opensync.orig/CMakeLists.txt opensync/CMakeLists.txt
--- opensync.orig/CMakeLists.txt 2008-06-05 16:03:53.000000000 +0800
+++ opensync/CMakeLists.txt 2008-06-05 18:01:27.000000000 +0800
@@ -51,6 +51,10 @@
  ADD_SUBDIRECTORY( tests )
 ENDIF ( CHECK_FOUND AND OPENSYNC_UNITTESTS )
 
+IF ( CMAKE_SYSTEM_NAME MATCHES SunOS )
+ SET( HAVE_SOLARIS 1 )
+ENDIF (CMAKE_SYSTEM_NAME MATCHES SunOS )
+
 ##############################################
 
 IF ( BUILD_DOCUMENTATION)
diff -ru opensync.orig/config.h.cmake opensync/config.h.cmake
--- opensync.orig/config.h.cmake 2008-06-05 16:03:53.000000000 +0800
+++ opensync/config.h.cmake 2008-06-05 18:01:52.000000000 +0800
@@ -19,6 +19,7 @@
 #cmakedefine OPENSYNC_TRACE
 
 #cmakedefine HAVE_FLOCK
+#cmakedefine HAVE_SOLARIS
 
 #define OPENSYNC_TESTDATA "${CMAKE_CURRENT_SOURCE_DIR}/tests/data"
 #cmakedefine OPENSYNC_UNITTESTS
diff -ru opensync.orig/tests/support.h opensync/tests/support.h
--- opensync.orig/tests/support.h 2008-06-05 16:03:53.000000000 +0800
+++ opensync/tests/support.h 2008-06-05 18:04:36.000000000 +0800
@@ -91,3 +91,9 @@
 osync_bool osync_testing_file_remove(const char *file);
 osync_bool osync_testing_file_chmod(const char *file, int mode);
 
+/* gdiff is GNU diff in Solaris */
+#ifdef HAVE_SOLARIS
+#define DIFF "gdiff"
+#else
+#define DIFF "diff"
+#endif
diff -ru opensync.orig/tests/sync-tests/check_filter.c opensync/tests/sync-tests/check_filter.c
--- opensync.orig/tests/sync-tests/check_filter.c 2008-06-05 16:03:53.000000000 +0800
+++ opensync/tests/sync-tests/check_filter.c 2008-06-05 18:07:50.000000000 +0800
@@ -118,8 +118,8 @@
  synchronize_once(engine, &error);
  osync_engine_finalize(engine, &error);
 
- fail_unless(!system("test \"x$(ls data1)\" = \"xtestdata\""), NULL);
- fail_unless(!system("test \"x$(ls data2)\" = \"xtestdata2\""), NULL);
+ fail_unless(!system("test \"x`(ls data1)`\" = \"xtestdata\""), NULL);
+ fail_unless(!system("test \"x`(ls data2)`\" = \"xtestdata2\""), NULL);
 
  destroy_testbed(testbed);
 }
@@ -162,7 +162,7 @@
  synchronize_once(engine, NULL);
  osync_engine_finalize(engine, &error);
 
- fail_unless(!system("test \"x$(diff -x \".*\" data1 data2)\" = \"x\""), NULL);
+ fail_unless(!system("test \"x`(" DIFF " -x \".*\" data1 data2)`\" = \"x\""), NULL);
 
  destroy_testbed(testbed);
 }
@@ -314,13 +314,13 @@
  osync_engine_finalize(engine, &error);
  osync_engine_unref(engine);
 
- fail_unless(!system("test \"x$(ls data1/testdata)\" = \"xdata1/testdata\""), NULL);
- fail_unless(!system("test \"x$(ls data1/testdata2)\" = \"xdata1/testdata2\""), NULL);
- fail_unless(!system("test \"x$(ls data1/testdata3)\" = \"xdata1/testdata3\""), NULL);
- fail_unless(!system("test \"x$(ls data1/vcard.vcf)\" = \"xdata1/vcard.vcf\""), NULL);
+ fail_unless(!system("test \"x`(ls data1/testdata)`\" = \"xdata1/testdata\""), NULL);
+ fail_unless(!system("test \"x`(ls data1/testdata2)`\" = \"xdata1/testdata2\""), NULL);
+ fail_unless(!system("test \"x`(ls data1/testdata3)`\" = \"xdata1/testdata3\""), NULL);
+ fail_unless(!system("test \"x`(ls data1/vcard.vcf)`\" = \"xdata1/vcard.vcf\""), NULL);
 
- fail_unless(!system("test \"x$(ls data2/testdata3)\" = \"xdata2/testdata3\""), NULL);
- fail_unless(!system("test \"x$(ls data2/vcard.vcf)\" = \"xdata2/vcard.vcf\""), NULL);
+ fail_unless(!system("test \"x`(ls data2/testdata3)`\" = \"xdata2/testdata3\""), NULL);
+ fail_unless(!system("test \"x`(ls data2/vcard.vcf)`\" = \"xdata2/vcard.vcf\""), NULL);
 
  destroy_testbed(testbed);
 }
@@ -403,9 +403,9 @@
 
  fail_unless(num_read == 1);
 
- fail_unless(!system("test \"x$(ls data1/testdata)\" = \"xdata1/testdata\""), NULL);
- fail_unless(!system("test \"x$(ls data1/testdata2)\" = \"xdata1/testdata2\""), NULL);
- fail_unless(!system("test \"x$(ls data2/testdata2)\" = \"xdata2/testdata2\""), NULL);
+ fail_unless(!system("test \"x`(ls data1/testdata)`\" = \"xdata1/testdata\""), NULL);
+ fail_unless(!system("test \"x`(ls data1/testdata2)`\" = \"xdata1/testdata2\""), NULL);
+ fail_unless(!system("test \"x`(ls data2/testdata2)`\" = \"xdata2/testdata2\""), NULL);
 
  destroy_testbed(testbed);
 }


-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Opensync-devel mailing list
Opensync-devel@...
https://lists.sourceforge.net/lists/listinfo/opensync-devel

[Daniel Gollub]

On Monday 09 June 2008 18:31:52 Jim Li wrote:
> Hi All,
>
> This patch fix two different behavior between Linux and Solaris:
> 1. The default shell of system on Solaris is sh which can't explain
> "test \"x$(ls data1)\" = \"xtestdata\"" correctly. we use `` to replace
> $ which can work well both for sh and bash which means it make unit test
> work well both on Linux and on Solaris

Actually we should get rid of all those system() calls, if possible.
Irene already pointed this out in some other testcases. I introduced some  
simple testhelper funciton which should replace those system calls:
http://opensync.org/changeset/3279

So instead of:
!system("test \"x$(ls data2/testdata3)\" = \"xdata2/testdata3\"")
we should do:
osync_testing_file_exists("data2/testdata3")

What do you think?

> 2. Solaris's diff doesn't support "-x " options, and it has gnu diff
> which name is gdiff. so we use a micro definition to fix this problem.

Nice! Maybe we should replace all the system("diff ...") calls with one
osync_testing_ helper function - e.g.: osync_testing_diff(const char *path1,
const char *path2) which does the system() call with the DIFF macro.

> Almost all unit test has this kind of problem. this patch is just a
> sample  patch only for check_filter. if you guys think it's fine, I'll
> deal with all of unit test for this issue, so many edit work ...
>
> welcome any suggestion and comments!
Thanks for looking in to this!
If you like i can implement those osync_testing_diff helper function. But it
should be quite trivial to implement - just let me know if you have questions
or need help. I hope with this set of helper function we can get rid of all
system() calls.

Don't hesitate to ask if there are outstanding question about those
testing-helpers or non-trival system() calls in the testcases. If you like i
can give you also write access to the SVN so you can commit directly your
changes. Just sent me a private mail with your prefered login.

best regards,
Daniel

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Opensync-devel mailing list
Opensync-devel@...
https://lists.sourceforge.net/lists/listinfo/opensync-devel

[Jim Li-3]

Daniel:

I rewrite the patch according to your great suggestion. please help me
to review it again.

Thanks

jim

> On Monday 09 June 2008 18:31:52 Jim Li wrote:
>  
>> Hi All,
>>
>> This patch fix two different behavior between Linux and Solaris:
>> 1. The default shell of system on Solaris is sh which can't explain
>> "test \"x$(ls data1)\" = \"xtestdata\"" correctly. we use `` to replace
>> $ which can work well both for sh and bash which means it make unit test
>> work well both on Linux and on Solaris
>>    
>
> Actually we should get rid of all those system() calls, if possible.
> Irene already pointed this out in some other testcases. I introduced some  
> simple testhelper funciton which should replace those system calls:
> http://opensync.org/changeset/3279
>
> So instead of:
> !system("test \"x$(ls data2/testdata3)\" = \"xdata2/testdata3\"")
> we should do:
> osync_testing_file_exists("data2/testdata3")
>
> What do you think?
>
>  
>> 2. Solaris's diff doesn't support "-x " options, and it has gnu diff
>> which name is gdiff. so we use a micro definition to fix this problem.
>>    
>
> Nice! Maybe we should replace all the system("diff ...") calls with one
> osync_testing_ helper function - e.g.: osync_testing_diff(const char *path1,
> const char *path2) which does the system() call with the DIFF macro.
>
>  
>> Almost all unit test has this kind of problem. this patch is just a
>> sample  patch only for check_filter. if you guys think it's fine, I'll
>> deal with all of unit test for this issue, so many edit work ...
>>
>> welcome any suggestion and comments!
>>    
> Thanks for looking in to this!
> If you like i can implement those osync_testing_diff helper function. But it
> should be quite trivial to implement - just let me know if you have questions
> or need help. I hope with this set of helper function we can get rid of all
> system() calls.
>
> Don't hesitate to ask if there are outstanding question about those
> testing-helpers or non-trival system() calls in the testcases. If you like i
> can give you also write access to the SVN so you can commit directly your
> changes. Just sent me a private mail with your prefered login.
>
> best regards,
> Daniel
>
> -------------------------------------------------------------------------
> Check out the new SourceForge.net Marketplace.
> It's the best place to buy or sell services for
> just about anything Open Source.
> http://sourceforge.net/services/buy/index.php
> _______________________________________________
> Opensync-devel mailing list
> Opensync-devel@...
> https://lists.sourceforge.net/lists/listinfo/opensync-devel
>  

diff -ru opensync.orig/CMakeLists.txt opensync/CMakeLists.txt
--- opensync.orig/CMakeLists.txt 2008-06-05 16:03:53.000000000 +0800
+++ opensync/CMakeLists.txt 2008-06-05 18:01:27.000000000 +0800
@@ -51,6 +51,10 @@
  ADD_SUBDIRECTORY( tests )
 ENDIF ( CHECK_FOUND AND OPENSYNC_UNITTESTS )
 
+IF ( CMAKE_SYSTEM_NAME MATCHES SunOS )
+ SET( HAVE_SOLARIS 1 )
+ENDIF (CMAKE_SYSTEM_NAME MATCHES SunOS )
+
 ##############################################
 
 IF ( BUILD_DOCUMENTATION)
diff -ru opensync.orig/config.h.cmake opensync/config.h.cmake
--- opensync.orig/config.h.cmake 2008-06-05 16:03:53.000000000 +0800
+++ opensync/config.h.cmake 2008-06-05 18:01:52.000000000 +0800
@@ -19,6 +19,7 @@
 #cmakedefine OPENSYNC_TRACE
 
 #cmakedefine HAVE_FLOCK
+#cmakedefine HAVE_SOLARIS
 
 #define OPENSYNC_TESTDATA "${CMAKE_CURRENT_SOURCE_DIR}/tests/data"
 #cmakedefine OPENSYNC_UNITTESTS
diff -ru opensync.orig/tests/support.c opensync/tests/support.c
--- opensync.orig/tests/support.c 2008-06-05 16:03:53.000000000 +0800
+++ opensync/tests/support.c 2008-06-12 20:54:56.000000000 +0800
@@ -619,3 +619,42 @@
  TODO: Do we have to care about this on Windows?! */
  return g_chmod(file, mode);
 }
+
+/*! @brief Copy files
+ *
+ * @param source source filename
+ * @param dest destination filename
+ * @returns TRUE on success, FALSE otherwise
+ *
+ */
+osync_bool osync_testing_file_copy(const char *source, const char *dest)
+{
+ gchar *cmd;
+ int ret;
+
+ cmd = g_strdup_printf ("cp %s %s", source, dest);
+ ret = system (cmd);
+ g_free (cmd);
+        
+ return ret;
+}
+
+/*! @brief Find differences between two files
+ *
+ * @param source source filename
+ * @param dest destination filename
+ * @returns TRUE on success, FALSE otherwise
+ *
+ */
+osync_bool osync_testing_diff(const char *file1, const char *file2)
+{
+        gchar *cmd;
+        int ret;
+
+        cmd = g_strdup_printf ("test \"x`(" DIFF " -x \".*\" %s %s)`\" = \"x\"", file1, file2);
+        ret = system (cmd);
+        g_free (cmd);
+
+        return ret;
+}
+
diff -ru opensync.orig/tests/support.h opensync/tests/support.h
--- opensync.orig/tests/support.h 2008-06-05 16:03:53.000000000 +0800
+++ opensync/tests/support.h 2008-06-18 17:40:13.000000000 +0800
@@ -90,4 +90,12 @@
 osync_bool osync_testing_file_exists(const char *file);
 osync_bool osync_testing_file_remove(const char *file);
 osync_bool osync_testing_file_chmod(const char *file, int mode);
+osync_bool osync_testing_file_copy(const char *source, const char *dest);
+osync_bool osync_testing_diff(const char *file1, const char *file2);
 
+/* gdiff is GNU diff in Solaris */
+#ifdef HAVE_SOLARIS
+#define DIFF "gdiff"
+#else
+#define DIFF "diff"
+#endif
diff -ru opensync.orig/tests/sync-tests/check_filter.c opensync/tests/sync-tests/check_filter.c
--- opensync.orig/tests/sync-tests/check_filter.c 2008-06-05 16:03:53.000000000 +0800
+++ opensync/tests/sync-tests/check_filter.c 2008-06-18 17:34:08.000000000 +0800
@@ -118,8 +118,8 @@
  synchronize_once(engine, &error);
  osync_engine_finalize(engine, &error);
 
- fail_unless(!system("test \"x$(ls data1)\" = \"xtestdata\""), NULL);
- fail_unless(!system("test \"x$(ls data2)\" = \"xtestdata2\""), NULL);
+ fail_unless(osync_testing_file_exists("data1/testdata"), NULL);
+ fail_unless(osync_testing_file_exists("data2/testdata2"), NULL);
 
  destroy_testbed(testbed);
 }
@@ -162,7 +162,7 @@
  synchronize_once(engine, NULL);
  osync_engine_finalize(engine, &error);
 
- fail_unless(!system("test \"x$(diff -x \".*\" data1 data2)\" = \"x\""), NULL);
+ fail_unless(!osync_testing_diff("data1", "data2"), NULL);
 
  destroy_testbed(testbed);
 }
@@ -314,13 +314,13 @@
  osync_engine_finalize(engine, &error);
  osync_engine_unref(engine);
 
- fail_unless(!system("test \"x$(ls data1/testdata)\" = \"xdata1/testdata\""), NULL);
- fail_unless(!system("test \"x$(ls data1/testdata2)\" = \"xdata1/testdata2\""), NULL);
- fail_unless(!system("test \"x$(ls data1/testdata3)\" = \"xdata1/testdata3\""), NULL);
- fail_unless(!system("test \"x$(ls data1/vcard.vcf)\" = \"xdata1/vcard.vcf\""), NULL);
-
- fail_unless(!system("test \"x$(ls data2/testdata3)\" = \"xdata2/testdata3\""), NULL);
- fail_unless(!system("test \"x$(ls data2/vcard.vcf)\" = \"xdata2/vcard.vcf\""), NULL);
+ fail_unless(osync_testing_file_exists("data1/testdata"), NULL);
+ fail_unless(osync_testing_file_exists("data1/testdata2"), NULL);
+ fail_unless(osync_testing_file_exists("data1/testdata3"), NULL);
+ fail_unless(osync_testing_file_exists("data1/vcard.vcf"), NULL);
+
+ fail_unless(osync_testing_file_exists("data2/testdata3"), NULL);
+ fail_unless(osync_testing_file_exists("data2/vcard.vcf"), NULL);
 
  destroy_testbed(testbed);
 }
@@ -357,7 +357,7 @@
 
  /* Synchronize once, delete a file, and synchronize again */
 
- fail_unless(!system("rm data1/file"), NULL);
+ fail_unless(!osync_testing_file_remove("data1/file"), NULL);
 
  synchronize_once(engine, NULL);
  mark_point();
@@ -403,9 +403,9 @@
 
  fail_unless(num_read == 1);
 
- fail_unless(!system("test \"x$(ls data1/testdata)\" = \"xdata1/testdata\""), NULL);
- fail_unless(!system("test \"x$(ls data1/testdata2)\" = \"xdata1/testdata2\""), NULL);
- fail_unless(!system("test \"x$(ls data2/testdata2)\" = \"xdata2/testdata2\""), NULL);
+ fail_unless(osync_testing_file_exists("data1/testdata"), NULL);
+ fail_unless(osync_testing_file_exists("data1/testdata2"), NULL);
+ fail_unless(osync_testing_file_exists("data2/testdata2"), NULL);
 
  destroy_testbed(testbed);
 }

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Opensync-devel mailing list
Opensync-devel@...
https://lists.sourceforge.net/lists/listinfo/opensync-devel

[Daniel Gollub]

On Wednesday 18 June 2008 11:46:48 Jim Li wrote:
> I rewrite the patch according to your great suggestion. please help me
> to review it again.
>
[...]

Looks, good to me. We may replace the osync_testing_file_copy() code with
something more portable (and secure?) later. Feel free to commit.

> +/*! @brief Copy files
> + *
> + * @param source source filename
> + * @param dest destination filename
> + * @returns TRUE on success, FALSE otherwise
> + *
> + */
> +osync_bool osync_testing_file_copy(const char *source, const char *dest)
> +{
> +       gchar *cmd;
> +       int ret;
> +
> +       cmd = g_strdup_printf ("cp %s %s", source, dest);
> +       ret = system (cmd);
> +       g_free (cmd);

(Is there a certain reason for putting space between the function name and the
arguments list? Not quite sure if this declared in the CODING file either...)

> +        
> +       return ret;



-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Opensync-devel mailing list
Opensync-devel@...
https://lists.sourceforge.net/lists/listinfo/opensync-devel

[Jim Li-3]

Daniel:

>> +/*! @brief Copy files
>> + *
>> + * @param source source filename
>> + * @param dest destination filename
>> + * @returns TRUE on success, FALSE otherwise
>> + *
>> + */
>> +osync_bool osync_testing_file_copy(const char *source, const char *dest)
>> +{
>> +       gchar *cmd;
>> +       int ret;
>> +
>> +       cmd = g_strdup_printf ("cp %s %s", source, dest);
>> +       ret = system (cmd);
>> +       g_free (cmd);
>>    
>
> (Is there a certain reason for putting space between the function name and the
> arguments list? Not quite sure if this declared in the CODING file either...)
>
>  

Not really! I'll modify this before commitment!

Thanks

Jim

>> +        
>> +       return ret;
>>    
>
>
>
> -------------------------------------------------------------------------
> Check out the new SourceForge.net Marketplace.
> It's the best place to buy or sell services for
> just about anything Open Source.
> http://sourceforge.net/services/buy/index.php
> _______________________________________________
> Opensync-devel mailing list
> Opensync-devel@...
> https://lists.sourceforge.net/lists/listinfo/opensync-devel
>  

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Opensync-devel mailing list
Opensync-devel@...
https://lists.sourceforge.net/lists/listinfo/opensync-devel