Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit54100f5

Browse files
committed
Add an enforcement mechanism for global object names in regression tests.
In commit18555b1 we tentatively established a rule that regressiontests should use names containing "regression" for databases, and namesstarting with "regress_" for all other globally-visible object names, soas to circumscribe the side-effects that "make installcheck" could haveon an existing installation.This commit adds a simple enforcement mechanism for that rule: if the codeis compiled with ENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS defined, itwill emit a warning (not an error) whenever a database, role, tablespace,subscription, or replication origin name is created that doesn't obey therule. Running one or more buildfarm members with that symbol definedshould be enough to catch new violations, at least in the regularregression tests. Most TAP tests wouldn't notice such warnings, butthat's actually fine because TAP tests don't execute against an existingserver anyway.Since it's already the case that running src/test/modules/ tests ininstallcheck mode is deprecated, we can use that as a home for teststhat seem unsafe to run against an existing server, such as tests thatmight have side-effects on existing roles. Document that (though thiscommit doesn't in itself make it any less safe than before).Update regress.sgml to define these restrictions more clearly, andto clean up assorted lack-of-up-to-date-ness in its descriptions ofthe available regression tests.Discussion:https://postgr.es/m/16638.1468620817@sss.pgh.pa.us
1 parentca129e5 commit54100f5

File tree

8 files changed

+172
-19
lines changed

8 files changed

+172
-19
lines changed

‎doc/src/sgml/regress.sgml

Lines changed: 86 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ make check
4747
<screen>
4848
<computeroutput>
4949
=======================
50-
All115 tests passed.
50+
All193 tests passed.
5151
=======================
5252
</computeroutput>
5353
</screen>
@@ -98,7 +98,7 @@ make MAX_CONNECTIONS=10 check
9898

9999
<para>
100100
To run the tests after installation (see <xref linkend="installation"/>),
101-
initialize a dataarea and start the
101+
initialize a datadirectory and start the
102102
server as explained in <xref linkend="runtime"/>, then type:
103103
<screen>
104104
make installcheck
@@ -116,10 +116,10 @@ make installcheck-parallel
116116

117117
<para>
118118
The tests will also transiently create some cluster-wide objects, such as
119-
rolesandtablespaces. These objects will have names beginning with
120-
<literal>regress_</literal>. Beware of using <literal>installcheck</literal>
121-
mode in installations that have any actual users or tablespaces named
122-
that way.
119+
roles, tablespaces,andsubscriptions. These objects will have names
120+
beginning with<literal>regress_</literal>. Beware of
121+
using <literal>installcheck</literal> mode with an installation that has
122+
any actual global objects namedthat way.
123123
</para>
124124
</sect2>
125125

@@ -130,7 +130,7 @@ make installcheck-parallel
130130
The <literal>make check</literal> and <literal>make installcheck</literal> commands
131131
run only the <quote>core</quote> regression tests, which test built-in
132132
functionality of the <productname>PostgreSQL</productname> server. The source
133-
distributionalsocontains additional test suites, most of them having
133+
distribution contains many additional test suites, most of them having
134134
to do with add-on functionality such as optional procedural languages.
135135
</para>
136136

@@ -146,9 +146,24 @@ make installcheck-world
146146
already-installed server, respectively, just as previously explained
147147
for <literal>make check</literal> and <literal>make installcheck</literal>. Other
148148
considerations are the same as previously explained for each method.
149-
Note that <literal>make check-world</literal> builds a separate temporary
150-
installation tree for each tested module, so it requires a great deal
151-
more time and disk space than <literal>make installcheck-world</literal>.
149+
Note that <literal>make check-world</literal> builds a separate instance
150+
(temporary data directory) for each tested module, so it requires more
151+
time and disk space than <literal>make installcheck-world</literal>.
152+
</para>
153+
154+
<para>
155+
On a modern machine with multiple CPU cores and no tight operating-system
156+
limits, you can make things go substantially faster with parallelism.
157+
The recipe that most PostgreSQL developers actually use for running all
158+
tests is something like
159+
<screen>
160+
make check-world -j8 >/dev/null
161+
</screen>
162+
with a <option>-j</option> limit near to or a bit more than the number
163+
of available cores. Discarding <systemitem>stdout</systemitem>
164+
eliminates chatter that's not interesting when you just want to verify
165+
success. (In case of failure, the <systemitem>stderr</systemitem>
166+
messages are usually enough to determine where to look closer.)
152167
</para>
153168

154169
<para>
@@ -166,8 +181,7 @@ make installcheck-world
166181
<itemizedlist>
167182
<listitem>
168183
<para>
169-
Regression tests for optional procedural languages (other than
170-
<application>PL/pgSQL</application>, which is tested by the core tests).
184+
Regression tests for optional procedural languages.
171185
These are located under <filename>src/pl</filename>.
172186
</para>
173187
</listitem>
@@ -184,6 +198,13 @@ make installcheck-world
184198
located in <filename>src/interfaces/ecpg/test</filename>.
185199
</para>
186200
</listitem>
201+
<listitem>
202+
<para>
203+
Tests for core-supported authentication methods,
204+
located in <filename>src/test/authentication</filename>.
205+
(See below for additional authentication-related tests.)
206+
</para>
207+
</listitem>
187208
<listitem>
188209
<para>
189210
Tests stressing behavior of concurrent sessions,
@@ -192,21 +213,36 @@ make installcheck-world
192213
</listitem>
193214
<listitem>
194215
<para>
195-
Tests of client programs under <filename>src/bin</filename>. See
196-
also <xref linkend="regress-tap"/>.
216+
Tests for crash recovery and physical replication,
217+
located in <filename>src/test/recovery</filename>.
218+
</para>
219+
</listitem>
220+
<listitem>
221+
<para>
222+
Tests for logical replication,
223+
located in <filename>src/test/subscription</filename>.
224+
</para>
225+
</listitem>
226+
<listitem>
227+
<para>
228+
Tests of client programs, located under <filename>src/bin</filename>.
197229
</para>
198230
</listitem>
199231
</itemizedlist>
200232

201233
<para>
202-
When using <literal>installcheck</literal> mode, these tests will destroy any
203-
existing databases named <literal>pl_regression</literal>,
204-
<literal>contrib_regression</literal>, <literal>isolation_regression</literal>,
205-
<literal>ecpg1_regression</literal>, or <literal>ecpg2_regression</literal>, as well as
206-
<literal>regression</literal>.
234+
When using <literal>installcheck</literal> mode, these tests will create
235+
and destroy test databases whose names
236+
include <literal>regression</literal>, for
237+
example <literal>pl_regression</literal>
238+
or <literal>contrib_regression</literal>. Beware of
239+
using <literal>installcheck</literal> mode with an installation that has
240+
any non-test databases named that way.
207241
</para>
208242

209243
<para>
244+
Some of these auxiliary test suites use the TAP infrastructure explained
245+
in <xref linkend="regress-tap"/>.
210246
The TAP-based tests are run only when PostgreSQL was configured with the
211247
option <option>--enable-tap-tests</option>. This is recommended for
212248
development, but can be omitted if there is no suitable Perl installation.
@@ -259,6 +295,17 @@ make check-world PG_TEST_EXTRA='kerberos ldap ssl'
259295
configuration are not run even if they are mentioned in
260296
<varname>PG_TEST_EXTRA</varname>.
261297
</para>
298+
299+
<para>
300+
In addition, there are tests in <filename>src/test/modules</filename>
301+
which will be run by <literal>make check-world</literal> but not
302+
by <literal>make installcheck-world</literal>. This is because they
303+
install non-production extensions or have other side-effects that are
304+
considered undesirable for a production installation. You can
305+
use <literal>make install</literal> and <literal>make
306+
installcheck</literal> in one of those subdirectories if you wish,
307+
but it's not recommended to do so with a non-test server.
308+
</para>
262309
</sect2>
263310

264311
<sect2>
@@ -737,6 +784,26 @@ make check PROVE_TESTS='t/001_test1.pl t/003_test3.pl'
737784
The TAP tests require the Perl module <literal>IPC::Run</literal>.
738785
This module is available from CPAN or an operating system package.
739786
</para>
787+
788+
<para>
789+
Generically speaking, the TAP tests will test the executables in a
790+
previously-installed installation tree if you say <literal>make
791+
installcheck</literal>, or will build a new local installation tree from
792+
current sources if you say <literal>make check</literal>. In either
793+
case they will initialize a local instance (data directory) and
794+
transiently run a server in it. Some of these tests run more than one
795+
server. Thus, these tests can be fairly resource-intensive.
796+
</para>
797+
798+
<para>
799+
It's important to realize that the TAP tests will start test server(s)
800+
even when you say <literal>make installcheck</literal>; this is unlike
801+
the traditional non-TAP testing infrastructure, which expects to use an
802+
already-running test server in that case. Some PostgreSQL
803+
subdirectories contain both traditional-style and TAP-style tests,
804+
meaning that <literal>make installcheck</literal> will produce a mix of
805+
results from temporary servers and the already-running test server.
806+
</para>
740807
</sect1>
741808

742809
<sect1 id="regress-coverage">

‎src/backend/commands/alter.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,12 @@ AlterObjectRename_internal(Relation rel, Oid objectId, const char *new_name)
274274
if (SearchSysCacheExists2(SUBSCRIPTIONNAME,MyDatabaseId,
275275
CStringGetDatum(new_name)))
276276
report_name_conflict(classId,new_name);
277+
278+
/* Also enforce regression testing naming rules, if enabled */
279+
#ifdefENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS
280+
if (strncmp(new_name,"regress_",8)!=0)
281+
elog(WARNING,"subscriptions created by regression test cases should have names starting with \"regress_\"");
282+
#endif
277283
}
278284
elseif (nameCacheId >=0)
279285
{

‎src/backend/commands/dbcommands.c

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -470,6 +470,16 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
470470
/* Note there is no additional permission check in this path */
471471
}
472472

473+
/*
474+
* If built with appropriate switch, whine when regression-testing
475+
* conventions for database names are violated. But don't complain during
476+
* initdb.
477+
*/
478+
#ifdefENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS
479+
if (IsUnderPostmaster&&strstr(dbname,"regression")==NULL)
480+
elog(WARNING,"databases created by regression test cases should have names including \"regression\"");
481+
#endif
482+
473483
/*
474484
* Check for db name conflict. This is just to give a more friendly error
475485
* message than "unique index violation". There's a race condition but
@@ -1008,6 +1018,15 @@ RenameDatabase(const char *oldname, const char *newname)
10081018
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
10091019
errmsg("permission denied to rename database")));
10101020

1021+
/*
1022+
* If built with appropriate switch, whine when regression-testing
1023+
* conventions for database names are violated.
1024+
*/
1025+
#ifdefENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS
1026+
if (strstr(newname,"regression")==NULL)
1027+
elog(WARNING,"databases created by regression test cases should have names including \"regression\"");
1028+
#endif
1029+
10111030
/*
10121031
* Make sure the new name doesn't exist. See notes for same error in
10131032
* CREATE DATABASE.

‎src/backend/commands/subscriptioncmds.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,15 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
357357
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
358358
(errmsg("must be superuser to create subscriptions"))));
359359

360+
/*
361+
* If built with appropriate switch, whine when regression-testing
362+
* conventions for subscription names are violated.
363+
*/
364+
#ifdefENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS
365+
if (strncmp(stmt->subname,"regress_",8)!=0)
366+
elog(WARNING,"subscriptions created by regression test cases should have names starting with \"regress_\"");
367+
#endif
368+
360369
rel=table_open(SubscriptionRelationId,RowExclusiveLock);
361370

362371
/* Check if name is used */

‎src/backend/commands/tablespace.c

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,15 @@ CreateTableSpace(CreateTableSpaceStmt *stmt)
307307
stmt->tablespacename),
308308
errdetail("The prefix \"pg_\" is reserved for system tablespaces.")));
309309

310+
/*
311+
* If built with appropriate switch, whine when regression-testing
312+
* conventions for tablespace names are violated.
313+
*/
314+
#ifdefENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS
315+
if (strncmp(stmt->tablespacename,"regress_",8)!=0)
316+
elog(WARNING,"tablespaces created by regression test cases should have names starting with \"regress_\"");
317+
#endif
318+
310319
/*
311320
* Check that there is no other tablespace by this name. (The unique
312321
* index would catch this anyway, but might as well give a friendlier
@@ -957,6 +966,15 @@ RenameTableSpace(const char *oldname, const char *newname)
957966
errmsg("unacceptable tablespace name \"%s\"",newname),
958967
errdetail("The prefix \"pg_\" is reserved for system tablespaces.")));
959968

969+
/*
970+
* If built with appropriate switch, whine when regression-testing
971+
* conventions for tablespace names are violated.
972+
*/
973+
#ifdefENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS
974+
if (strncmp(newname,"regress_",8)!=0)
975+
elog(WARNING,"tablespaces created by regression test cases should have names starting with \"regress_\"");
976+
#endif
977+
960978
/* Make sure the new name doesn't exist */
961979
ScanKeyInit(&entry[0],
962980
Anum_pg_tablespace_spcname,

‎src/backend/commands/user.c

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,15 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
326326
stmt->role),
327327
errdetail("Role names starting with \"pg_\" are reserved.")));
328328

329+
/*
330+
* If built with appropriate switch, whine when regression-testing
331+
* conventions for role names are violated.
332+
*/
333+
#ifdefENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS
334+
if (strncmp(stmt->role,"regress_",8)!=0)
335+
elog(WARNING,"roles created by regression test cases should have names starting with \"regress_\"");
336+
#endif
337+
329338
/*
330339
* Check the pg_authid relation to be certain the role doesn't already
331340
* exist.
@@ -1212,6 +1221,15 @@ RenameRole(const char *oldname, const char *newname)
12121221
newname),
12131222
errdetail("Role names starting with \"pg_\" are reserved.")));
12141223

1224+
/*
1225+
* If built with appropriate switch, whine when regression-testing
1226+
* conventions for role names are violated.
1227+
*/
1228+
#ifdefENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS
1229+
if (strncmp(newname,"regress_",8)!=0)
1230+
elog(WARNING,"roles created by regression test cases should have names starting with \"regress_\"");
1231+
#endif
1232+
12151233
/* make sure the new name doesn't exist */
12161234
if (SearchSysCacheExists1(AUTHNAME,CStringGetDatum(newname)))
12171235
ereport(ERROR,

‎src/backend/replication/logical/origin.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1238,6 +1238,15 @@ pg_replication_origin_create(PG_FUNCTION_ARGS)
12381238
name),
12391239
errdetail("Origin names starting with \"pg_\" are reserved.")));
12401240

1241+
/*
1242+
* If built with appropriate switch, whine when regression-testing
1243+
* conventions for replication origin names are violated.
1244+
*/
1245+
#ifdefENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS
1246+
if (strncmp(name,"regress_",8)!=0)
1247+
elog(WARNING,"replication origins created by regression test cases should have names starting with \"regress_\"");
1248+
#endif
1249+
12411250
roident=replorigin_create(name);
12421251

12431252
pfree(name);

‎src/test/modules/README

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,13 @@ intended for testing PostgreSQL and/or to serve as example code. The extensions
66
here aren't intended to be installed in a production server and aren't suitable
77
for "real work".
88

9+
Furthermore, while you can do "make install" and "make installcheck" in
10+
this directory or its children, it is NOT ADVISABLE to do so with a server
11+
containing valuable data. Some of these tests may have undesirable
12+
side-effects on roles or other global objects within the tested server.
13+
"make installcheck-world" at the top level does not recurse into this
14+
directory.
15+
916
Most extensions have their own pg_regress tests or isolationtester specs. Some
1017
are also used by tests elsewhere in the tree.
1118

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp