Re: Getting rid of regression test input/ and output/ files
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Date: 2021-12-19T21:08:07Z
Lists: pgsql-hackers
Attachments
- v1-0001-add-getenv-command.patch (text/x-diff) patch v1-0001
- v1-0002-use-env-vars-in-tests.patch (text/x-diff) patch v1-0002
- v1-0003-move-scripts-to-normal-dirs.patch (text/x-diff) patch v1-0003
- v1-0004-remove-useless-infrastructure.patch (text/x-diff) patch v1-0004
I wrote: > This led me to wonder why we couldn't get rid of that entire > mechanism in favor of some less-painful way of getting that > information into the scripts. If we had the desired values in > psql variables, we could do what we need easily, for example ... Here's some fleshed-out patches for this. 0001 adds the \getenv command to psql; now with documentation and a simple regression test. 0002 tweaks pg_regress to export the needed values as environment variables, and modifies the test scripts to use those variables. (For ease of review, this patch modifies the scripts in-place, and then 0003 will move them.) A few comments on this: * I didn't see any value in exporting @testtablespace@ as a separate variable; we might as well let the test script know how to construct that path name. * I concluded that the right way to handle the concatenation issue is *not* to rely on SQL literal concatenation, but to use psql's \set command to concatenate parts of a string. In particular this gives us a clean way to handle quoting/escaping rules in the places where a pathname has to be embedded in some larger string, such as a function body. The golden rule for that seems to be "use one \set per level of quoting". I believe this code is now fairly proof against situations that would completely break the existing way of doing things, such as pathnames with quotes or backslashes in them. (It's hard to test the embedded-quote case, because that breaks the Makefiles too, but I did get through the regression tests with a path including a backslash.) * There are a couple of places where the existing tests involve substituting a path name into expected query output or error messages. This technique cannot handle that, but we have plenty of prior art for dealing with such cases. I changed file_fdw to use a filter function to hide the pathnames in EXPLAIN output, and tweaked create_function_0 to show only an edited version of an error message (this is based on a similar case in infinite_recurse.sql). 0003 simply "git mv"'s the scripts and output files into place as normal not-requiring-editing files. Be careful to "make clean" before applying this, else you may have conflicts with the target files already being present. Also, while you can run the tests between 0003 and 0004, don't do "make clean" in this state or the hacky EXTRA_CLEAN rules in dblink and file_fdw will remove files you want. 0004 finally removes the no-longer-needed infrastructure in pg_regress and the makefiles. (BTW, as far as I can find, the MSVC scripts have no provisions for cleaning these generated files?) There's some refactoring that could be done afterwards, for example there seems little reason for dblink's paths.sql to continue to exist as a separate script. But it seemed best for this patch series to convert the scripts as mechanically as possible. I'm fairly pleased with how this came out. I think these scripts will be *much* easier to maintain in this form. Updating the output/*.source files was always a major pain in the rear, since you couldn't just copy results/ files to them. Comments? regards, tom lane
Commits
-
Merge dblink's paths test script into its main test.
- 0e6e7f0806b2 15.0 landed
-
Remove dynamic translation of regression test scripts, step 2.
- dc9c3b0ff214 15.0 landed
-
Remove dynamic translation of regression test scripts, step 1.
- d1029bb5a26c 15.0 landed
-
Add a \getenv command to psql.
- 33d3eeadb21d 15.0 landed