Opened 9 months ago

Closed 2 months ago

#5683 closed defect (duplicate)

32bit test fails on ST_SnapToGrid test

Reported by: strk Owned by: pramsey
Priority: blocker Milestone: PostGIS 3.5.0
Component: postgis Version: master
Keywords: regression Cc:

Description

See ​https://gitlab.com/postgis/postgis/-/jobs/6292825666#L4140

Last successful build by gitalb-ci was [dd88d87cfc3b7b7b7f059171fb090ee4b2db8836/git]

Failure is:

 regress/core/snaptogrid .. failed (diff expected obtained: /tmp/pgis_reg/test_75_diff)
-----------------------------------------------------------------------------
--- /builds/postgis/postgis/regress/core/snaptogrid_expected	2024-03-01 10:15:36.238767575 +0000
+++ /tmp/pgis_reg/test_75_out	2024-03-01 10:18:46.730484609 +0000
@@ -7,4 +7,4 @@
 POLYGON((0 0,10 0,10 10,0 10,0 0))|BOX(0 0,10 10)
 #5241|MULTIPOLYGON EMPTY
 #5241|MULTIPOLYGON(((9 9,9 1,1 1,2 4,7 7,9 9)))
-#5448|LINESTRING(0 0,1 1)
+#5448|LINESTRING(0 0,1 1,1 1,1 1)
-----------------------------------------------------------------------------

Referenced ticket is #5448

The test is also present in stable-3.4 branch where it runs successfully, making this a regression issue. For this reason I'm marking it as a blocker.

Change History (11)

comment:1 by pramsey, 9 months ago

I just reviewed the whole diff (git diff dd88d87cfc3b7b7b7f059171fb090ee4b2db8836) to head and there is no relevant code touched. I'm going to add a cunit test that exercises the same code line and see if that also fails.

comment:2 by pramsey, 9 months ago

Hm, I see gitlab-ci doesn't run the cunit tests in liblwgeom, so I cannot see if my unit test works. Probably the set-up script doesn't install the cunit support before configure/build?

comment:3 by pramsey, 9 months ago

Does the environment differ in stable-3.4? I just went through the whole code line and there is no difference between 3.4 and main.

comment:4 by robe, 9 months ago

Looks like it's running the master tests now on cunit and I see this on master

  Test: test_rect_tree_intersects_tree ...NOTICE: /builds/postgis/postgis/liblwgeom/ptarray.c:449 - ptarray_segmentize2d: Too many segments required (1.000000e+101)

and assert failure on test_grid_in_place

 Test: test_grid_in_place ...FAILED
    1. /builds/postgis/postgis/liblwgeom/cunit/cu_misc.c:170  - CU_ASSERT_STRING_EQUAL(wkt_result,wkt_norm)

Strangely enough 3.4 branch seems to be succeeding even on the cunit but at a glance not seeing cunit tests running or perhaps I missed, cause it does seem to have cunit installed.

comment:5 by Regina Obe <lr@…>, 9 months ago

In 3ee973b/git:

Install libcunit1 on gitlab 32-bit
Also explicitly test liblwgeom cunit as is done on master
References #5683 for PostGIS 3.4.3

comment:6 by robe, 9 months ago

apologies that thing I thought was a failure

is expected result and shows on stable-3.4 as well

 Test: test_lwgeom_segmentize2d ...NOTICE: /builds/postgis/postgis/liblwgeom/ptarray.c:449 - ptarray_segmentize2d: Too many segments required (1.000000e+101)

But interestingly stable-3.4 32-bit gitlab run is not showing the test_grid_in_place error. That is passing with flying colors as is the rest of the tests

https://gitlab.com/postgis/postgis/-/jobs/6398823690#L733

I thought maybe it was the debian version, but master and stable-3.4 are running debian bookworm

But I do notice the cu_misc.c grid tests change in master which I presume pramsey added to exercise what is causing the postgresql failures.

master has an extra test:

	do_grid_test(
		"LINESTRING(0 0,1 1,1 1,1 1)",
		"LINESTRING(0 0,1 1)",
		0.001
	);

I'm going to add this to 3.4 as well to see if gitlab test32 is still a happy camper.

comment:7 by Regina Obe <lr@…>, 9 months ago

In 9246ff3/git:

Add failing master 32bit test. References #5683

comment:8 by robe, 9 months ago

okay it passed in 3.4 — https://gitlab.com/postgis/postgis/-/jobs/6398945512#L726 so must be something else

comment:9 by robe, 9 months ago

I was reading strk's notice wrong. I was thinking he was blaming pramsey's big fat commit of https://trac.osgeo.org/postgis/changeset/dd88d87cfc3b7b7b7f059171fb090ee4b2db8836/git

No that was the last successful one.

So whatever the cause of this happened after January 14th (after the above).

Sadly we had a long period of both the test and test32 failing because of stupid defined but not used notices,

/builds/postgis/postgis/liblwgeom/profile.h:5:15: error: 'cpu_time_used' defined but not used [-Werror=unused-variable]
    5 | static double cpu_time_used;
      |               ^~~~~~~~~~~~~
/builds/postgis/postgis/liblwgeom/profile.h:4:28: error: 'end_time' defined but not used [-Werror=unused-variable]
    4 | static clock_t start_time, end_time;
      |                            ^~~~~~~~
/builds/postgis/postgis/liblwgeom/profile.h:4:16: error: 'start_time' defined but not used [-Werror=unused-variable]
    4 | static clock_t start_time, end_time;

that evidentally we were blissfully ignoring then followed by another storm, of "Regina is sloppy" errors

  272 |       snprintf(errbuf, errbuflen, _("ERROR: column map file specifies a DBF field name \"%s\" which is longer than 10 characters"), map->dbffieldnames[curmapsize]);
      |                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/builds/postgis/postgis/loader/shpcommon.h:24:19: note: in definition of macro '_'
   24 | #define _(String) String
      |                   ^~~~~~
/builds/postgis/postgis/loader/shpcommon.c:272:52: note: format string is defined here
  272 |       snprintf(errbuf, errbuflen, _("ERROR: column map file specifies a DBF field name \"%s\" which is longer than 10 characters"), map->dbffieldnames[curmapsize]);

which we were also blissfully ignoring.

, then followed by a storm of "What is strk doing with topology?" and why is he letting all the bots turn red.

Until we finally get to @strk commit which fixes the last of those sloppy errors leaving us with a green on 64-bit test and a red test32

https://gitlab.com/postgis/postgis/-/jobs/6292825666

https://gitlab.com/postgis/postgis/-/commit/07a73c0f2c5cb07e33fd64dadd24fac35efdc0a5

So issue is somewhere after January 14th and on or before February 29th.

comment:10 by strk, 7 months ago

I guess #5724 is a duplicate of this ?

comment:11 by robe, 2 months ago

Resolution: duplicate
Status: newclosed

yah I think so

Note: See TracTickets for help on using tickets.