make check false success

Started by strkalmost 9 years ago5 messageshackers
Jump to latest
#1strk
strk@keybit.net

I noticed that the `check` Makefile rule imported by PGXS is giving
a success exit code even when it is unsupported.

The attached patch fixes that.

--strk;

() Free GIS & Flash consultant/developer
/\ https://strk.kbt.io/services.html

Attachments:

0001-Make-sure-make-check-fails-when-it-cannot-be-run.patchtext/x-diff; charset=us-asciiDownload+1-1
#2Robert Haas
robertmhaas@gmail.com
In reply to: strk (#1)
Re: make check false success

On Thu, Jun 1, 2017 at 10:18 AM, Sandro Santilli <strk@kbt.io> wrote:

I noticed that the `check` Makefile rule imported by PGXS is giving
a success exit code even when it is unsupported.

The attached patch fixes that.

Hmm. I'm not 100% sure that the existing behavior is wrong.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3strk
strk@keybit.net
In reply to: Robert Haas (#2)
Re: make check false success

On Fri, Jun 02, 2017 at 08:20:25AM -0400, Robert Haas wrote:

On Thu, Jun 1, 2017 at 10:18 AM, Sandro Santilli <strk@kbt.io> wrote:

I noticed that the `check` Makefile rule imported by PGXS is giving
a success exit code even when it is unsupported.

The attached patch fixes that.

Hmm. I'm not 100% sure that the existing behavior is wrong.

Why not ? The caller is attempting to make an unsupported target,
how's that different from calling `make unexistent` ?

--strk;

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: strk (#3)
Re: make check false success

Sandro Santilli <strk@kbt.io> writes:

On Fri, Jun 02, 2017 at 08:20:25AM -0400, Robert Haas wrote:

On Thu, Jun 1, 2017 at 10:18 AM, Sandro Santilli <strk@kbt.io> wrote:

I noticed that the `check` Makefile rule imported by PGXS is giving
a success exit code even when it is unsupported.

Hmm. I'm not 100% sure that the existing behavior is wrong.

Why not ? The caller is attempting to make an unsupported target,
how's that different from calling `make unexistent` ?

The key question here is whether you break recursive behavior.
We wouldn't be happy if, for instance, contrib/Makefile had to know
exactly which subdirectories it was safe to recurse into when doing
"make check". Now, that doesn't apply directly because we're not
using the PGXS logic when we do that --- but it seems entirely
possible to me that a PGXS-using module might be a subdirectory
of some larger project, so that it would not want to fail just
because it had no tests it could run.

A related point is that if you try "make check" in a subdirectory
that doesn't have REGRESS defined at all, you don't get an error
(and this patch wouldn't change that). It would be pretty inconsistent
to throw an error if REGRESS is defined but not if it isn't, IMO.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Robert Haas
robertmhaas@gmail.com
In reply to: strk (#3)
Re: make check false success

On Mon, Jun 5, 2017 at 5:23 AM, Sandro Santilli <strk@kbt.io> wrote:

Why not ? The caller is attempting to make an unsupported target,
how's that different from calling `make unexistent` ?

That's a good point, but what Tom wrote is along the lines of my
concerns also, especially his last paragraph about REGRESS not being
defined at all. I think we have a convention that 'make check'
succeeds if it runs all of the tests, even if the set of all tests
happens to be the empty set.

What was your motivation for wanting this changed in the first place?
It seems like either behavior could be more convenient for someone,
depending on the context.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers