Обсуждение: Remove backend warnings from SSL tests

Поиск
Список
Период
Сортировка

Remove backend warnings from SSL tests

От
Daniel Gustafsson
Дата:
When looking at a patch in the CFBot I realized that the SSL tests generate
backend warnings under ENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS due to roles
and databases not following the regression test naming convention.  While not
impacting the tested functionality, it's pretty silly to have warnings in the
test logs which can be avoided since those can throw off users debugging a test
failure.

The attached renames all roles with a regress_ prefix and databases with a
regression_ prefix to match the convention, and regenerates the certificates to
match.  With this I get a clean warning-free testrun.  There are no functional
changes included, just changed names (and comments to match).

--
Daniel Gustafsson


Вложения

Re: Remove backend warnings from SSL tests

От
Aleksander Alekseev
Дата:
Hi,

> When looking at a patch in the CFBot I realized that the SSL tests generate
> backend warnings under ENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS

Good catch. I can confirm that the patch corrects the named WARNINGs
appearing with:

CPPFLAGS="-DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS"

There are plenty of similar warnings left however.

Before:

```
$ grep -r WARNING ./build/ 2>/dev/null | grep 'regression test cases
should have names' | wc -l
463
```

After:

```
$ grep -r WARNING ./build/ 2>/dev/null | grep 'regression test cases
should have names' | wc -l
403
```

Maybe we should address them too. In order to prevent this from
happening in the future perhaps we should start throwing ERRORs
instead of a WARNINGs and make sure this is tested by cfbot.

Alternatively we could get rid of
ENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS entirely since its practical
value seems to be debatable.

The patch was added to the nearest commitfest [1].

Thoughts?

[1]: https://commitfest.postgresql.org/44/4451/

-- 
Best regards,
Aleksander Alekseev



Re: Remove backend warnings from SSL tests

От
Tom Lane
Дата:
Aleksander Alekseev <aleksander@timescale.com> writes:
>> When looking at a patch in the CFBot I realized that the SSL tests generate
>> backend warnings under ENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS

> Good catch. I can confirm that the patch corrects the named WARNINGs
> appearing with:
> CPPFLAGS="-DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS"
> There are plenty of similar warnings left however.

Yeah.  We have not worried about making TAP tests clean under this
restriction, because the point of it is to limit the hazards of
running "make installcheck" against a cluster containing useful data.
TAP tests always use one-off test clusters, so there is no hazard
to guard against.

If we wanted to extend the rule to TAP tests as well, I think we'd
have to upgrade the WARNING to an ERROR, because otherwise we'll
never find all the violations.  Not clear to me that it's worth
the trouble, though.  And it's definitely not worth the trouble to
fix only one TAP suite.

> Alternatively we could get rid of
> ENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS entirely since its practical
> value seems to be debatable.

Strong -1 on that, for the reason given above.

            regards, tom lane



Re: Remove backend warnings from SSL tests

От
Tom Lane
Дата:
I wrote:
> Aleksander Alekseev <aleksander@timescale.com> writes:
>> Alternatively we could get rid of
>> ENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS entirely since its practical
>> value seems to be debatable.

> Strong -1 on that, for the reason given above.

Perhaps an alternative could be to expend some more sweat on the
mechanism, so that it's actually enforced (with ERROR) during
"make installcheck", but not in "make check" or TAP tests?
I'm not sure how to make that work exactly.

            regards, tom lane