[PATCH] Regression tests in windows ignore white space
In the following thread I discovered that my new regression tests worked
perfectly on windows, but when they were run on linux they failed.
/messages/by-id/CAApHDvo_YCiPYGDz07MpX9o6EGg=3mmyJTb0ysPTwoTg3c=Tvw@mail.gmail.com
After looking at pg_regress to see which options it passes to diff I
discovered that it passes -w on windows to ignore ALL white space.
The attached simple patch changes this so that it only ignores carriage
returns. It does this by passing --strip-trailing-cr to diff instead of -w.
This should help us few developers who use windows to get our white space
correct in out expected results so that the tests also pass on non windows
platforms.
Regards
David Rowley
Attachments:
pg_regress_win32_diff_options.patchapplication/octet-stream; name=pg_regress_win32_diff_options.patchDownload
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index e8dec46..a3a6540 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -66,16 +66,17 @@ static char *shellprog = SHELLPROG;
#endif
/*
- * On Windows we use -w in diff switches to avoid problems with inconsistent
- * newline representation. The actual result files will generally have
- * Windows-style newlines, but the comparison files might or might not.
+ * Since the expected result files most likely terminate lines with a line feed
+ * and windows will generate the actual outputs with both carriage return and
+ * line feed then we need to pass slightly different options to diff for windows
+ * builds.
*/
#ifndef WIN32
const char *basic_diff_opts = "";
const char *pretty_diff_opts = "-C3";
#else
-const char *basic_diff_opts = "-w";
-const char *pretty_diff_opts = "-w -C3";
+const char *basic_diff_opts = "--strip-trailing-cr";
+const char *pretty_diff_opts = "--strip-trailing-cr -C3";
#endif
/* options settable from command line */
On Thu, Dec 26, 2013 at 3:02 PM, David Rowley <dgrowleyml@gmail.com> wrote:
In the following thread I discovered that my new regression tests worked
perfectly on windows, but when they were run on linux they failed./messages/by-id/CAApHDvo_YCiPYGDz07MpX9o6EGg=3mmyJTb0ysPTwoTg3c=Tvw@mail.gmail.com
After looking at pg_regress to see which options it passes to diff I
discovered that it passes -w on windows to ignore ALL white space.The attached simple patch changes this so that it only ignores carriage
returns. It does this by passing --strip-trailing-cr to diff instead of -w.
This should help us few developers who use windows to get our white space
correct in out expected results so that the tests also pass on non windows
platforms.
I also faced some similar issue few days back and tried to search a
bit for some simpler option, but couldn't spend too much time.
I think it would be good if we can make it work without -w option.
When I tried with your patch on windows, it results into following:
============== running regression test queries ==============
test tablespace ... diff: unrecognized option `--strip-trailing-cr
'
diff: Try `diff --help' for more information.
diff command failed with status 2: "diff --strip-trailing-cr
"e:/../postgresql/src/test/regress/expected/tablespace.out"
"e:/../postgresql/src/test/regress/results/tablespace.out" >
"e:/../postgresql/src/test/regress/results/tablespace.out.diff"
"
Which version of diff you are using?
Version of diff on my m/c is:
diff - GNU diffutils version 2.7
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sun, Dec 29, 2013 at 2:29 AM, Amit Kapila <amit.kapila16@gmail.com>wrote:
On Thu, Dec 26, 2013 at 3:02 PM, David Rowley <dgrowleyml@gmail.com>
wrote:In the following thread I discovered that my new regression tests worked
perfectly on windows, but when they were run on linux they failed./messages/by-id/CAApHDvo_YCiPYGDz07MpX9o6EGg=3mmyJTb0ysPTwoTg3c=Tvw@mail.gmail.com
After looking at pg_regress to see which options it passes to diff I
discovered that it passes -w on windows to ignore ALL white space.The attached simple patch changes this so that it only ignores carriage
returns. It does this by passing --strip-trailing-cr to diff instead of-w.
This should help us few developers who use windows to get our white space
correct in out expected results so that the tests also pass on nonwindows
platforms.
I also faced some similar issue few days back and tried to search a
bit for some simpler option, but couldn't spend too much time.
I think it would be good if we can make it work without -w option.When I tried with your patch on windows, it results into following:
============== running regression test queries ==============
test tablespace ... diff: unrecognized option
`--strip-trailing-cr
'
diff: Try `diff --help' for more information.
diff command failed with status 2: "diff --strip-trailing-cr
"e:/../postgresql/src/test/regress/expected/tablespace.out"
"e:/../postgresql/src/test/regress/results/tablespace.out" >
"e:/../postgresql/src/test/regress/results/tablespace.out.diff"
"Which version of diff you are using?
Version of diff on my m/c is:
diff - GNU diffutils version 2.7
I had a bit of a look around on the git repository for diffutils and I've
found at least part of the commit which introduced --strip-trailing-cr
http://git.savannah.gnu.org/cgit/diffutils.git/commit/?id=eefb9adae1642dcb0e2ac523c79998f466e94e77
Although here I can only see that they've added the command line args and
not the actual code which implements stripping the carriage returns. Going
by that it seems that was added back in 2001, but the version you're using
is a bit older than than, it seems 2.7 is 19 years old!
http://git.savannah.gnu.org/cgit/diffutils.git/refs/tags
I'm on 2.8.7 which is only 4 years old.
I know we don't normally go with bleeding edge, but I wondering if we could
make the minimum supported diff version at least 2.8 (at least for windows)
which was released in 2002.
Regards
David Rowley
Show quoted text
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
On Sun, Dec 29, 2013 at 2:06 AM, David Rowley <dgrowleyml@gmail.com> wrote:
On Sun, Dec 29, 2013 at 2:29 AM, Amit Kapila <amit.kapila16@gmail.com>
wrote:When I tried with your patch on windows, it results into following:
============== running regression test queries ==============
test tablespace ... diff: unrecognized option
`--strip-trailing-cr
'
Which version of diff you are using?Version of diff on my m/c is:
diff - GNU diffutils version 2.7I had a bit of a look around on the git repository for diffutils and I've
found at least part of the commit which introduced --strip-trailing-crhttp://git.savannah.gnu.org/cgit/diffutils.git/commit/?id=eefb9adae1642dcb0e2ac523c79998f466e94e77
Although here I can only see that they've added the command line args and
not the actual code which implements stripping the carriage returns. Going
by that it seems that was added back in 2001, but the version you're using
is a bit older than than, it seems 2.7 is 19 years old!
For Windows build, I am using whatever latest Git provides rather than
downloading
individual components which might not be good, but I find it
convenient. The latest
Git (1.8.4) download on windows still provides 2.7, which is the
reason I am on older
version. However I agree that it is better to use latest version.
http://git.savannah.gnu.org/cgit/diffutils.git/refs/tags
I'm on 2.8.7 which is only 4 years old.
I know we don't normally go with bleeding edge, but I wondering if we could
make the minimum supported diff version at least 2.8 (at least for windows)
which was released in 2002.
I have checked that for some of the other components like bison, flex,
ActiveState TCL we specify minimum version required, but not for diff.
http://www.postgresql.org/docs/devel/static/install-windows-full.html
+1, for minimum diff version as 2.8.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Dec 30, 2013 at 6:02 PM, Amit Kapila <amit.kapila16@gmail.com>wrote:
On Sun, Dec 29, 2013 at 2:06 AM, David Rowley <dgrowleyml@gmail.com>
wrote:On Sun, Dec 29, 2013 at 2:29 AM, Amit Kapila <amit.kapila16@gmail.com>
wrote:When I tried with your patch on windows, it results into following:
============== running regression test queries ==============
test tablespace ... diff: unrecognized option
`--strip-trailing-cr
'
Which version of diff you are using?Version of diff on my m/c is:
diff - GNU diffutils version 2.7I had a bit of a look around on the git repository for diffutils and I've
found at least part of the commit which introduced --strip-trailing-crhttp://git.savannah.gnu.org/cgit/diffutils.git/commit/?id=eefb9adae1642dcb0e2ac523c79998f466e94e77
Although here I can only see that they've added the command line args and
not the actual code which implements stripping the carriage returns.Going
by that it seems that was added back in 2001, but the version you're
using
is a bit older than than, it seems 2.7 is 19 years old!
For Windows build, I am using whatever latest Git provides rather than
downloading
individual components which might not be good, but I find it
convenient. The latest
Git (1.8.4) download on windows still provides 2.7, which is the
reason I am on older
version. However I agree that it is better to use latest version.
It looks like the diff version I'm using is from msys and msys is what is
in my search path rather than the git\bin path. To be honest I didn't
realise that git for windows came with bison and flex, (at least I see
bison.exe and flex.exe in the git\bin path.. I don't remember me putting
them there)
I don't seem to even have git\bin in my %path% environment variable at all,
so all those tools are being picked up from the msys path. I'd need to
remind myself about the msys install process, but since we're already
saying in the docs that msys should be installed, then would the fix for
this not just be as simple as my patch plus a note in the docs to say to
ensure msys\bin occurs before git\bin in the %path% environment var,
minimum supported diff version is 2.8.
Did you install msys? if so does it have a later version of diff?
Regards
David Rowley
Show quoted text
http://git.savannah.gnu.org/cgit/diffutils.git/refs/tags
I'm on 2.8.7 which is only 4 years old.
I know we don't normally go with bleeding edge, but I wondering if we
could
make the minimum supported diff version at least 2.8 (at least for
windows)
which was released in 2002.
I have checked that for some of the other components like bison, flex,
ActiveState TCL we specify minimum version required, but not for diff.
http://www.postgresql.org/docs/devel/static/install-windows-full.html+1, for minimum diff version as 2.8.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
On Mon, Dec 30, 2013 at 11:08 AM, David Rowley <dgrowleyml@gmail.com> wrote:
On Mon, Dec 30, 2013 at 6:02 PM, Amit Kapila <amit.kapila16@gmail.com>
wrote:For Windows build, I am using whatever latest Git provides rather than
downloading
individual components which might not be good, but I find it
convenient. The latest
Git (1.8.4) download on windows still provides 2.7, which is the
reason I am on older
version. However I agree that it is better to use latest version.It looks like the diff version I'm using is from msys and msys is what is in
my search path rather than the git\bin path. To be honest I didn't realise
that git for windows came with bison and flex, (at least I see bison.exe and
flex.exe in the git\bin path.. I don't remember me putting them there)
Yes, git for windows came with bison,flex, etc required for PG build.
I don't seem to even have git\bin in my %path% environment variable at all,
so all those tools are being picked up from the msys path. I'd need to
remind myself about the msys install process, but since we're already saying
in the docs that msys should be installed,
I think it is one of the ways of set up on Windows, not every user
uses from msys.
then would the fix for this not
just be as simple as my patch plus a note in the docs to say to ensure
msys\bin occurs before git\bin in the %path% environment var, minimum
supported diff version is 2.8.
To be honest, I am also not fully aware of the project policy in such
a matter where we have to mention the minimum version of dependent
executable (diff). I request some senior members to please let us know
whether we can make PG dependent on diff version >= 2.8 (atleast
for windows), as there are some of the options which are not available
in older versions.
Another option could be that if version of diff available is >= 2.8, then
use '--strip-trailing-cr', else use existing option '-w', but not
sure if it will
serve the purpose completely.
Did you install msys? if so does it have a later version of diff?
No.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers