Обсуждение: Remove backend warnings from SSL tests
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
Вложения
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
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
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