Better context for "TAP tests not enabled" error message

Started by Kevin Burkeover 4 years ago6 messages
#1Kevin Burke
kevin@burke.dev
1 attachment(s)

I got the error message in the subject and was unsure how to continue; I
didn't see any hits for the error message on the mailing list, and it was
hard to determine from the context around the error in the
Makefile.global.in about the best way to solve the problem.

This patch amends the error message to give help to the user.

I ran "make check" at the top level with this patch enabled and 209 tests
passed. I also ran "make check" in src/test/ssl without TAP enabled and
verified that I got the new error message. I also verified that compiling
with --enable-tap-tests fixes the error in question.

This patch does not include regression tests.

Another way to fix this issue could be to put the exact text of the error
message in the documentation or the wiki, with instructions on how to fix
it - the first thing I did was punch the error message into Google, if a
match for the error message came up with instructions on how to fix it,
that would also help.

This is the first patch that I've submitted to Postgres, I believe that
I've followed the guidelines on the patch submission page, but please let
me know if I missed anything.

Kevin

--
Kevin Burke
phone: 925-271-7005 | kevin.burke.dev

Attachments:

0001-Makefile-provide-better-help-if-TAP-tests-are-not-en.patchapplication/octet-stream; name=0001-Makefile-provide-better-help-if-TAP-tests-are-not-en.patchDownload
From 85333bc7b635dcf2434d961eeeff870c8562ad12 Mon Sep 17 00:00:00 2001
From: Kevin Burke <kevin@burke.dev>
Date: Fri, 1 Oct 2021 11:49:26 -0700
Subject: [PATCH] Makefile: provide better help if TAP tests are not enabled

I previously got stuck at this error message when trying to run the
SSL tests. The error message does not provide a whole lot of context
about how to fix the problem. This provides a little bit more context
with a likely solution.
---
 src/Makefile.global.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 6e2f224cc4..f14f799d96 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -476,7 +476,7 @@ cd $(srcdir) && \
 endef
 
 else
-prove_installcheck = @echo "TAP tests not enabled"
+prove_installcheck = @echo "TAP tests not enabled. Maybe you need to configure with --enable-tap-tests"
 prove_check = $(prove_installcheck)
 endif
 
-- 
2.32.0

#2Daniel Gustafsson
daniel@yesql.se
In reply to: Kevin Burke (#1)
Re: Better context for "TAP tests not enabled" error message

On 2 Oct 2021, at 02:09, Kevin Burke <kevin@burke.dev> wrote:

This patch amends the error message to give help to the user.

I think this makes sense, and is in line with Rachels patch [0]/messages/by-id/CADJcwiVL20955HCNzDqz9BEDr6A77pz6-nac5sbZVvhAEMijLg@mail.gmail.com a few days ago
that I plan on pushing; small error hints which wont get in the way of
established developers, but which can help new developers onboard onto our tree
and processes around it.

--
Daniel Gustafsson https://vmware.com/

[0]: /messages/by-id/CADJcwiVL20955HCNzDqz9BEDr6A77pz6-nac5sbZVvhAEMijLg@mail.gmail.com

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Daniel Gustafsson (#2)
Re: Better context for "TAP tests not enabled" error message

Daniel Gustafsson <daniel@yesql.se> writes:

On 2 Oct 2021, at 02:09, Kevin Burke <kevin@burke.dev> wrote:
This patch amends the error message to give help to the user.

I think this makes sense,

+1. I'd take out the "Maybe"; the diagnosis seems pretty certain.

regards, tom lane

#4Kevin Burke
kevin@burke.dev
In reply to: Tom Lane (#3)
1 attachment(s)
Re: Better context for "TAP tests not enabled" error message

Updated patch that removes the "Maybe"

--
Kevin Burke
phone: 925-271-7005 | kevin.burke.dev

On Sat, Oct 2, 2021 at 11:19 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Show quoted text

Daniel Gustafsson <daniel@yesql.se> writes:

On 2 Oct 2021, at 02:09, Kevin Burke <kevin@burke.dev> wrote:
This patch amends the error message to give help to the user.

I think this makes sense,

+1. I'd take out the "Maybe"; the diagnosis seems pretty certain.

regards, tom lane

Attachments:

0001-Makefile-provide-better-help-if-TAP-tests-are-not-en.patchapplication/octet-stream; name=0001-Makefile-provide-better-help-if-TAP-tests-are-not-en.patchDownload
From 1788c9dc8da41a2eebc80201c22ad0dfb97b6f96 Mon Sep 17 00:00:00 2001
From: Kevin Burke <kevin@burke.dev>
Date: Fri, 1 Oct 2021 11:49:26 -0700
Subject: [PATCH] Makefile: provide better help if TAP tests are not enabled

I previously got stuck at this error message when trying to run the
SSL tests. The error message does not provide a whole lot of context
about how to fix the problem. This provides a little bit more context
with a likely solution.
---
 src/Makefile.global.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index e4fd7b5290..a1da1ea4ee 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -476,7 +476,7 @@ cd $(srcdir) && \
 endef
 
 else
-prove_installcheck = @echo "TAP tests not enabled"
+prove_installcheck = @echo "TAP tests not enabled. Try configuring with --enable-tap-tests"
 prove_check = $(prove_installcheck)
 endif
 
-- 
2.32.0

#5Daniel Gustafsson
daniel@yesql.se
In reply to: Kevin Burke (#4)
Re: Better context for "TAP tests not enabled" error message

On 3 Oct 2021, at 00:39, Kevin Burke <kevin@burke.dev> wrote:

Updated patch that removes the "Maybe"

Thanks, I’ll take care of this tomorrow along with Rachels patch.

./daniel

#6Daniel Gustafsson
daniel@yesql.se
In reply to: Daniel Gustafsson (#5)
Re: Better context for "TAP tests not enabled" error message

On 3 Oct 2021, at 01:27, Daniel Gustafsson <daniel@yesql.se> wrote:

On 3 Oct 2021, at 00:39, Kevin Burke <kevin@burke.dev> wrote:

Updated patch that removes the "Maybe"

Thanks, I’ll take care of this tomorrow along with Rachels patch.

I was off-by-one on date, but it's been pushed to master now. Thanks!

--
Daniel Gustafsson https://vmware.com/