Re: Regression tests - Mailing list pgsql-odbc

On Wed, Jan 28, 2015 at 8:42 PM, Heikki Linnakangas
<hlinnakangas@vmware.com> wrote:
> Hi,
>
> Last weekend I tried to run the regression tests on a different system than
> what I usually use, but it failed because the system didn't have pg_regress
> installed. It had a PostgreSQL server running, and it had libpq and libpq
> header files, but pg_regress and the rest of the PGXS system was not
> included in those OS packages.
>
> It was easy enough to install the right packages (apt-get install ...), but
> it's nevertheless a bit silly that we depend on PGXS and pg_regress. When I
> wrote the regression suite, I actually had to jump through some hoops to use
> pg_regress in the first place. There's the ugly hack where "make
> installcheck" creates the .sql files that just run the test binaries.
>
> Using pg_regress also means that the ODBC.ini file must agree with the
> current PGHOST etc. settings. Otherwise psql will fail to connect even if
> odbc.ini is correct, or worse, it will connect to a different server.
>
> I've also been trying to make it easier to run the regression tests on
> Windows. Using psql and pg_regress is a bit of pain there, and again it
> requires that you have those installed on the build machine, which isn't a
> given.
>
> For these reasons, I propose to refactor the way the regression tests are
> launched. I wrote a small C utility to basically replace pg_regress. It runs
> the test programs, compares their outputs with the expected outputs, and
> runs "diff" to create regression.diffs file if they don't match. It produces
> TAP-compatible output, which is nice because you can use a frontend like
> perl 'prove' program to pretty-print it with colors and everything, and you
> can easily integrate it with tools like Jenkins. But it's also quite
> human-readable without any such tools.
>
> Patch attached. Any objections?
No real objections, I am fine with removing the dependency to PGXS.

> The second patch adds a .vcproj file for MSVC, for running the regression
> tests, and a -RegressionTests option to the BuildAll command to run the
> regression tests after building the drivers. There are a few regression
> failures on Windows, so you won't get a clean pass, but at least it's much
> easier to run the tests with this.

Some remarks about the first patch:
1) test/reset-db and test/runsuite are automatically generated. They
should be added in test/.gitignore. On Windows their .exe need to have
entries as well.
2) Tests will fail if test/results does not exist, and that's the case
of a raw tree. You can either create an empty folder test/results with
a .gitignore containing "*", or enforce the creation of this folder if
it does not exist in test/. TBH, the latter is better that having an
empty folder results/ to keep the code tree clean. This problem is of
course the same on all platforms.
3) test/expected/parse.out still has its header line, making the tests fail.
4) There is a typo in this Makefile target:
+runsuit: runsuite.c
5) Shouldn't you use strrchr instead of strchr?
+       /* if the input is a plain test name, construct the binary
name from it */
+       if (strchr(in, DIR_SEP) == NULL)
+       {
+               strcpy(testname, in);
+               sprintf(binname, "src%c%s-test", DIR_SEP, in);
+               return;
+       }
6) I would imagine strrchr here as well:
+       while (*s != '\0' && (s = strchr(s + 1, DIR_SEP)) != NULL)
+               basename = s + 1;
7) s/regression.diff/regression.diffs in runsuite.c
8) It doesn't matter much as process exits quickly, but [coverity]
closing fd in slurpfile() before calling bailout() would be nice
[/coverity].
9) sampletables.sql needs to have 1 SQL per line to have reset-db work
correctly. I don't mind much about this restriction but it seems to me
that this restriction meritates a comment at the top of
sampletables.sql.

About the second patch:
1) winbuild/readme.txt should have a bit of documentation
Regards,
--
Michael