quotearg test (was: Re: locale_charset() on MacOS X)

[Bruno Haible]

Hi Eric,

>   - POSIX [1] does not specify the character encoding of the "C" locale.
>     It could be US-ASCII or any extension of it, such as ISO-8859-1 or
>     UTF-8.
> ...
> Let's fix the testsuites.

Here's a proposed patch for the testsuite failure currently seen on
MacOS X 10.5:

  test-quotearg.h:53: assertion failed
  FAIL: test-quotearg-simple

Eric, each time I have to dig into this test, I find it extremely
hard to understand, because you need a debugger in order to see
what strings are passed where. Without a debugger, just from looking
at the source code, I have to grok the *entire* source code before I
can understand anything.

Today I also find it not extensible. If I wanted to add extra strings
(str1a, str1b, etc.) only for locale_quoting_style and clocale_quoting_style,
I cannot do it. I would be forced to add these extra strings to *all*
result_strings and result_groups, because the code is data driven and
there is the implicit assumption that the shape of the data must be the
same across all tests. This means, it is extremely hard to add conditionals.

In other words,

    data[] = { data1. ... data7 };
    for all indices in data[]
      test (data[i]);

is so much harder to extend than

    text (data1);
    ...
    text (data7);

In summary, I find that data-driven style is a bad programming style for
tests, and it should be rewritten in functional style (such as e.g. in
test-argp.c).

Concretely, this would mean
  - keep the function compare(),
  - change the function compare_strings to take 9 individual arguments
    instead of a 'struct result_strings *',
  - split it into two functions, one that corresponds to the case
    ascii_only==true, one for the case ascii_only==false.
  - in main(), invert the scope of the loops over the styles and over
    the data: Put the loop over different data elements outside the loop
    over the styles; otherwise it is impossible to add extra data only
    for one style but not for the others.

What do you think about it?


2012-01-28  Bruno Haible  <address@hidden>

        quotearg: Fix test failure on MacOS X 10.5.
        * tests/test-quotearg-simple.c: Include localcharset.h.
        (main): If the locale encoding is not ASCII, bypass the tests of
        locale_quoting_style and clocale_quoting_style.

--- tests/test-quotearg-simple.c.orig   Sun Jan 29 03:56:48 2012
+++ tests/test-quotearg-simple.c        Sun Jan 29 03:55:25 2012
@@ -27,6 +27,7 @@
 #include <stdlib.h>
 #include <string.h>
 
+#include "localcharset.h"
 #include "progname.h"
 #include "macros.h"
 
@@ -245,12 +246,20 @@
   for (i = literal_quoting_style; i <= clocale_quoting_style; i++)
     {
       set_quoting_style (NULL, (enum quoting_style) i);
-      compare_strings (use_quotearg_buffer, &results_g[i].group1, ascii_only);
-      compare_strings (use_quotearg, &results_g[i].group2, ascii_only);
-      if (i == c_quoting_style)
-        compare_strings (use_quote_double_quotes, &results_g[i].group2,
-                         ascii_only);
-      compare_strings (use_quotearg_colon, &results_g[i].group3, ascii_only);
+      if (!(i == locale_quoting_style || i == clocale_quoting_style)
+          || (strcmp (locale_charset (), "ASCII") == 0
+              || strcmp (locale_charset (), "ANSI_X3.4-1968") == 0))
+        {
+          compare_strings (use_quotearg_buffer, &results_g[i].group1,
+                           ascii_only);
+          compare_strings (use_quotearg, &results_g[i].group2,
+                           ascii_only);
+          if (i == c_quoting_style)
+            compare_strings (use_quote_double_quotes, &results_g[i].group2,
+                             ascii_only);
+          compare_strings (use_quotearg_colon, &results_g[i].group3,
+                           ascii_only);
+        }
     }
 
   set_quoting_style (NULL, literal_quoting_style);