Horton

Thread

Commits

  1. Document PG_TEST_NOCLEAN in TAP test README

  2. Doc: improve README files associated with TAP tests.

  1. Add additional information to src/test/ssl/README

    Kevin Burke <kevin@burke.dev> — 2021-10-30T03:10:08Z

    Hi,
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.

Kevin

  2. Re: Add additional information to src/test/ssl/README

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-10-30T14:14:46Z

    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

  3. Re: Add additional information to src/test/ssl/README

    Kevin Burke <kevin@burke.dev> — 2021-10-30T17:46:09Z

    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"

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'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
>

  4. Re: Add additional information to src/test/ssl/README

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-10-30T18:12:53Z

    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"

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

  5. Re: Add additional information to src/test/ssl/README

    Daniel Gustafsson <daniel@yesql.se> — 2021-10-30T18:41:45Z

    
> 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/

  6. Re: Add additional information to src/test/ssl/README

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-10-30T20:48:40Z

    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

  7. Re: Add additional information to src/test/ssl/README

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-10-31T18:28:35Z

    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

  8. Re: Add additional information to src/test/ssl/README

    Daniel Gustafsson <daniel@yesql.se> — 2021-10-31T20:15:43Z

    > 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/

  9. Re: Add additional information to src/test/ssl/README

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-10-31T22:13:52Z

    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

  10. Re: Add additional information to src/test/ssl/README

    Kevin Burke <kevin@burke.dev> — 2021-11-02T17:00:53Z

    The patch looks great. Thanks!

Kevin

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

> 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
>

  11. Re: Add additional information to src/test/ssl/README

    Daniel Gustafsson <daniel@yesql.se> — 2021-11-12T15:16:10Z

    > 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/

  12. Re: Add additional information to src/test/ssl/README

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-11-12T15:21:21Z

    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

  13. Re: Add additional information to src/test/ssl/README

    Daniel Gustafsson <daniel@yesql.se> — 2021-11-12T20:59:30Z

    > 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/