shp2pgsql cunit tests failing on Windows

[robe]

I thoughts this was just something wrong about the way I configured things on winnie's 32-bit runs, but now I have the same issue on my local 64-bit runs.

     CUnit - A unit testing framework for C - Version 2.1-2
     http://cunit.sourceforge.net/


Suite: Shapefile Loader File shp2pgsql Test
  Test: test_ShpLoaderCreate() ...passed
  Test: test_ShpLoaderDestroy() ...Makefile:76: recipe for target 'check' failed
make[2]: *** [check] Segmentation fault
make[2]: Leaving directory '/projects/postgis/branches/2.4/loader/cunit'
Makefile:157: recipe for target 'check' failed
make[1]: *** [check] Error 2
make[1]: Leaving directory '/projects/postgis/branches/2.4/loader'
GNUmakefile:16: recipe for target 'check' failed
make: *** [check] Error 1

thought I ticketed this one before but guess not because it started happening suddenly one day on winnie's 32-bit runs so assume it was just a configuration problem.

I should also add that winnie is the only bot that runs these tests. I had assumed the reason was that these were gui tests, but I see no reference to shp2pgsql-gui in these so not sure why the other bots shouldn't be running them as well.

[robe]

okay seemed to be crashing for me in the colmap_clean function. Remarking that out made the error go away. Though I can't figure out what I changed in my local to suddently start triggering the error.

Anyway I'm seeing a bunch of questionable code in shpcommon.c

colmap_dbf_by_pg(colmap *map, const char *pgname)
{
  int i;
  for (i=0; i<map->size; ++i)

shouldn't the counter be incremented after be i++ and not before?

anyway it disagrees with the colmap_clean code which has the counter incrementing after as I would expect it to.

colmap_clean(colmap *map)
{
	int i;

	if (map->size)
	{
	   for (i = 0; i < map->size; i++)

[robe]

Committed fixes at r15675 and also reenabled 32-bit testing on winnie. If it fixes winnie's issues, I'll backport to 2.3.4.

[robe]

sadly this did not fix winnie's 32-bit issue.

Suite: Shapefile Loader File shp2pgsql Test
  Test: test_ShpLoaderCreate() ...passed
  Test: test_ShpLoaderDestroy() ...make[2]: *** [Makefile:76: check] Segmentation fault
make[2]: Leaving directory '/projects/postgis/branches/2.4/loader/cunit'
make[1]: *** [Makefile:157: check] Error 2
make[1]: Leaving directory '/projects/postgis/branches/2.4/loader'
make: *** [GNUmakefile:16: check] Error 1

e:\jenkins\postgis>exit 2 
Build step 'Execute Windows batch command' marked build as failure
An attempt to send an e-mail to empty list of recipients, ignored.
Triggering a new build of PG_ED_STOP
Finished: FAILURE

[robe]

anyone else want to try these tests. It seems something else is going on with destroy I don't understand because even my local is failing sometimes. I'm really tempted to cheat and just remark out the contents of this function in cu_shp2pgsql.c

void test_ShpLoaderDestroy(void)
{
	ShpLoaderDestroy(loader_state);
}

So I can make sure 32-bit is working correctly in more important ways.

[robe]

In 15678:

initialize state->num_fields and state->pgfieldtypes on shape file create otherwise cunit ends up with a garbage huge number for state->num_fields which I think causes the crash I am seeing sometimes.

References #3839 for PostGIS 2.4.0

[robe]

fingers crossed looks like my change in r15678 fixed this issue for winnie's 32-bit runs. She's passed thru one with no errors. I'll backport the change to PostGIS 2.3.4 if everyone approves.

Key changes -- fixed colmap increment as it would be missing the first column the way it was written. Probably not a commonly used feature so perhaps no one noticed. Other issue was initializing the state->pgfieldtypes and state->num_fields. Without that looks like at least on 32-bit goes bonkers and sets num_fields to a huge number like 91233897987. did it sometimes on my 64-bit run to, or perhaps always and depending on state of my workstation would crash.

[robe]

as strk commented ++i , i++ are the same in for (....) so no need to back port that. The code is just inconsistent is all.

The only really issue is the lack of initialization of the variables.

[robe]

changed at r15681 to require all bots to test loader cunit

[robe]

just as a note of side history, these tests were originally turned off for no-gui as a result of this thread

​https://lists.osgeo.org/pipermail/postgis-devel/2010-December/011007.html

On Thu, Dec 16, 2010 at 11:33:21AM +0000, Mark Cave-Ayland wrote:
> strk wrote:
> 
> >Going on with my SUBDIR-based makefiles I stubled upon
> >loader/cunit dir (running 'make check' under loader).
> >
> >W/out my change, those unit test don't run at all, is
> >that expected ? Running the tests give lots of failures.
> >
> >So, are those tests meant to be run and should thus be fixed ?
> >Or should they be just NOT run (and thus probably dropped) ?
> 
> Hmmm it looks like it was something Mark (Leslie) was working on, though 
> I get GTK reference errors so they would have absolutely no chance of 
> running unless ./configure was run with --with-gui.

Alright then, as of r6414 'make check' under loader/  only runs the test
when --with-gui was passed. 

When I'm finished with the top-level-dir pass this will be true also
from the top-level dir...

--strk;

but given all the tests pass on the bots by enabling I think the comment is no longer valid.

[robe]

In 15682:

always enable loader cunit tests
References #3839

[robe]

In 15683:

initialize state->num_fields and state->pgfieldtypes so doesn't cause intermittent crashes on windows
Closes #3839 for PostGIS 2.3.4