Add additional information to src/test/ssl/README

Kevin Burke <kevin@burke.dev> writes:

I've been trying to run the SSL tests against my branch and that was
tougher than expected because I didn't realize that additional output was
being saved that I couldn't see - it wasn't even getting to the part where
it could run the tests. This patch adds a note to the README explaining
where to look if you get an error.

That's a generic thing about every one of our TAP tests, so documenting
it in that one README doesn't seem like an appropriate answer. Would
it have helped you to explain it in

https://www.postgresql.org/docs/devel/regress-tap.html

?

regards, tom lane

I probably would not have looked there honestly; I was working in the
terminal and had the source code right there. Could we put a link to that
page in the README?

"For more information on Postgres's TAP tests, see the docs:
https://www.postgresql.org/docs/devel/regress-tap.html&quot;

Kevin

On Sat, Oct 30, 2021 at 7:14 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Kevin Burke <kevin@burke.dev> writes:

I probably would not have looked there honestly; I was working in the
terminal and had the source code right there.

Yeah, that was kind of what I thought.

"For more information on Postgres's TAP tests, see the docs:
https://www.postgresql.org/docs/devel/regress-tap.html&quot;

This doesn't seem tremendously useful, as we'd still have to duplicate
it everywhere. A quick search finds these TAP suites that have
associated README files:

$ for ft in `find . -name t`

do
d=`dirname $ft`
if [ -e $d/README ]; then
echo $d/README
fi
done

./src/bin/pg_amcheck/README
./src/test/authentication/README
./src/test/kerberos/README
./src/test/ldap/README
./src/test/modules/test_misc/README
./src/test/modules/test_pg_dump/README
./src/test/modules/libpq_pipeline/README
./src/test/recovery/README
./src/test/ssl/README
./src/test/subscription/README

The ones under modules/ can probably be excluded, as they're not
there to provide usage directions; but that still leaves us with
seven to touch. I'd be inclined to add just one sentence to the
boilerplate text these use, along the lines of

"If the tests fail, examining the logs left behind in tmp_check/log/
may be helpful."

regards, tom lane

On 30 Oct 2021, at 20:12, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I'd be inclined to add just one sentence to the boilerplate text these use,
along the lines of

"If the tests fail, examining the logs left behind in tmp_check/log/
may be helpful."

Wouldn't it make more sense to start collecting troubleshooting advice in
src/test/perl/README and instead refer to that in the boilerplate? I notice
that we don't document for example PG_TEST_NOCLEAN anywhere (which admittedly
is my fault), a trubleshooting section in that file would be a good fit.

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

Daniel Gustafsson <daniel@yesql.se> writes:

Wouldn't it make more sense to start collecting troubleshooting advice in
src/test/perl/README and instead refer to that in the boilerplate? I notice
that we don't document for example PG_TEST_NOCLEAN anywhere (which admittedly
is my fault), a trubleshooting section in that file would be a good fit.

Yeah, we could try to move all the repetitive stuff to there. I was a bit
allergic to the idea of having README files refer to webpages, because you
might be offline --- but referencing a different README doesn't have that
issue.

regards, tom lane

I wrote:

Daniel Gustafsson <daniel@yesql.se> writes:

Wouldn't it make more sense to start collecting troubleshooting advice in
src/test/perl/README and instead refer to that in the boilerplate? I notice
that we don't document for example PG_TEST_NOCLEAN anywhere (which admittedly
is my fault), a trubleshooting section in that file would be a good fit.

Yeah, we could try to move all the repetitive stuff to there. I was a bit
allergic to the idea of having README files refer to webpages, because you
might be offline --- but referencing a different README doesn't have that
issue.

Here's a quick stab at rearranging src/test/perl/README so that the
initial section is all how-to-run-the-tests info, and advice about
writing new tests starts after that. Your point about PG_TEST_NOCLEAN
could be added into the initial section.

I'd envision adding something like

"See src/test/perl/README for more info about running these tests."

to the boilerplate "Running the tests" section in src/test/ssl/README
and its cohorts, but I didn't bother with that yet.

regards, tom lane

On 31 Oct 2021, at 19:28, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I wrote:

Daniel Gustafsson <daniel@yesql.se> writes:

Wouldn't it make more sense to start collecting troubleshooting advice in
src/test/perl/README and instead refer to that in the boilerplate? I notice
that we don't document for example PG_TEST_NOCLEAN anywhere (which admittedly
is my fault), a trubleshooting section in that file would be a good fit.

Yeah, we could try to move all the repetitive stuff to there. I was a bit
allergic to the idea of having README files refer to webpages, because you
might be offline --- but referencing a different README doesn't have that
issue.

Here's a quick stab at rearranging src/test/perl/README so that the
initial section is all how-to-run-the-tests info, and advice about
writing new tests starts after that. Your point about PG_TEST_NOCLEAN
could be added into the initial section.

That's pretty much just what I was thinking, definitely +1 on this patch.

I'd envision adding something like

"See src/test/perl/README for more info about running these tests."

to the boilerplate "Running the tests" section in src/test/ssl/README
and its cohorts, but I didn't bother with that yet.

Sounds good.

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

Daniel Gustafsson <daniel@yesql.se> writes:

On 31 Oct 2021, at 19:28, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Here's a quick stab at rearranging src/test/perl/README so that the
initial section is all how-to-run-the-tests info, and advice about
writing new tests starts after that. Your point about PG_TEST_NOCLEAN
could be added into the initial section.

That's pretty much just what I was thinking, definitely +1 on this patch.

Done that way; feel free to add more material to perl/README.

regards, tom lane

The patch looks great. Thanks!

Kevin

On Sun, Oct 31, 2021 at 3:13 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

On 31 Oct 2021, at 23:13, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Done that way; feel free to add more material to perl/README.

Attached is a small addition mentioning PG_TEST_NOCLEAN

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

Daniel Gustafsson <daniel@yesql.se> writes:

Attached is a small addition mentioning PG_TEST_NOCLEAN

Maybe could use a bit of copy-editing, say

Data directories will also be left behind for analysis when a test fails;
they are named according to the test filename. But if the environment
variable PG_TEST_NOCLEAN is set, data directories will be retained
regardless of test status.

regards, tom lane

On 12 Nov 2021, at 16:21, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Daniel Gustafsson <daniel@yesql.se> writes:

Attached is a small addition mentioning PG_TEST_NOCLEAN

Maybe could use a bit of copy-editing, say

Data directories will also be left behind for analysis when a test fails;
they are named according to the test filename. But if the environment
variable PG_TEST_NOCLEAN is set, data directories will be retained
regardless of test status.

Agreed, that reads better. Pushed with those changes.

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