[PATCH] Backport perl tests for pg_upgrade from 322becb60
Hello! In previous discussion (https://www.postgresql.org/message-id/flat/6b05291c-f252-4fae-317d-b50dba69c311%40inbox.ru) On 05.07.2022 22:08, Justin Pryzby wrote: I'm not sure if anyone is interested in patching test.sh in backbranches. I'm not sure, but there may be more interest to backpatch the conversion to TAP (322becb60). As far as i understand from this thread: https://www.postgresql.org/message-id/flat/Yox1ME99GhAemMq1%40paquier.xyz, the aim of the perl version for the pg_upgrade tests is to achieve equality of dumps for most cross-versions cases. If so this is the significant improvement as previously in test.sh resulted dumps retained unequal and the user was asked to eyeball them manually during cross upgrades between different major versions. So, the backport of the perl tests also seems preferable to me. In the attached patch has a backport to REL_13_STABLE. It has been tested from 9.2+ and give zero dumps diff from 10+. Also i've backported b34ca595, ba15f161, 95c3a195, 4c4eaf3d and b3983888 to reduce changes in the 002_pg_upgrade.pl and b33259e2 to fix an error when upgrading from 9.6. Dumps filtering and some other changes were backported from thread https://www.postgresql.org/message-id/flat/Yox1ME99GhAemMq1%40paquier.xyz too. Would be very grateful for comments and suggestions before trying to do this for other versions. I have a some question concerning patch tester. As Justin said it fails on non-master patches since it tries to apply all the *.patch files to the master branch, one after another. For branches other than master, I suggest to name the patches *.txt or similar. So, i made a .txt extension for patch, but i would really like to set a patch tester on it. Is there any way to do this? With best regards, -- Anton A. Melnikov Postgres Professional: http://www.postgrespro.com The Russian Postgres Companydiff --git a/src/bin/pg_upgrade/.gitignore b/src/bin/pg_upgrade/.gitignore index 9edea5c98f..05200a09f1 100644 --- a/src/bin/pg_upgrade/.gitignore +++ b/src/bin/pg_upgrade/.gitignore @@ -1,11 +1,4 @@ /pg_upgrade # Generated by test suite -/pg_upgrade_internal.log -/analyze_new_cluster.sh -/delete_old_cluster.sh -/analyze_new_cluster.bat -/delete_old_cluster.bat -/reindex_hash.sql -/loadable_libraries.txt /log/ /tmp_check/ diff --git a/src/bin/pg_upgrade/Makefile b/src/bin/pg_upgrade/Makefile index 0360c37bf9..105199f182 100644 --- a/src/bin/pg_upgrade/Makefile +++ b/src/bin/pg_upgrade/Makefile @@ -28,6 +28,10 @@ OBJS = \ override CPPFLAGS := -DDLSUFFIX=\"$(DLSUFFIX)\" -I$(srcdir) -I$(libpq_srcdir) $(CPPFLAGS) LDFLAGS_INTERNAL += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport) +# required for 002_pg_upgrade.pl +REGRESS_SHLIB=$(abs_top_builddir)/src/test/regress/regress$(DLSUFFIX) +export REGRESS_SHLIB + all: pg_upgrade pg_upgrade: $(OBJS) | submake-libpq submake-libpgport submake-libpgfeutils @@ -44,22 +48,10 @@ uninstall: clean distclean maintainer-clean: rm -f pg_upgrade$(X) $(OBJS) - rm -rf analyze_new_cluster.sh delete_old_cluster.sh log/ tmp_check/ \ - loadable_libraries.txt reindex_hash.sql \ - pg_upgrade_dump_globals.sql \ - pg_upgrade_dump_*.custom pg_upgrade_*.log - -# When $(MAKE) is present, make automatically infers that this is a -# recursive make. which is not actually what we want here, as that -# e.g. prevents output synchronization from working (as make thinks -# that the subsidiary make knows how to deal with that itself, but -# we're invoking a shell script that doesn't know). Referencing -# $(MAKE) indirectly avoids that behaviour. -# See https://www.gnu.org/software/make/manual/html_node/MAKE-Variable.html#MAKE-Variable -NOTSUBMAKEMAKE=$(MAKE) + rm -rf log/ tmp_check/ -check: test.sh all temp-install - MAKE=$(NOTSUBMAKEMAKE) $(with_temp_install) bindir=$(abs_top_builddir)/tmp_install/$(bindir) EXTRA_REGRESS_OPTS="$(EXTRA_REGRESS_OPTS)" $(SHELL) $< +check: + $(prove_check) -# installcheck is not supported because there's no meaningful way to test -# pg_upgrade against a single already-running server +installcheck: + $(prove_installcheck) diff --git a/src/bin/pg_upgrade/TESTING b/src/bin/pg_upgrade/TESTING index e69874b42d..200ce9d92b 100644 --- a/src/bin/pg_upgrade/TESTING +++ b/src/bin/pg_upgrade/TESTING @@ -2,25 +2,22 @@ THE SHORT VERSION - On non-Windows machines, you can execute the testing process -described below by running +described below by running the following command in this directory: make check -in this directory. This will run the shell script test.sh, performing -an upgrade from the version in this source tree to a new instance of -the same version. -To test an upgrade from a different version, you must have a built -source tree for the old version as well as this version, and you -must have done "make install" for both versions. Then do: +This will run the TAP tests to run pg_upgra
Re: [PATCH] Backport perl tests for pg_upgrade from 322becb60
Hello! On 09.12.2022 08:19, Michael Paquier wrote: On Mon, Aug 01, 2022 at 01:02:21AM +0300, Anton A. Melnikov wrote: As far as i understand from this thread: https://www.postgresql.org/message-id/flat/Yox1ME99GhAemMq1%40paquier.xyz, the aim of the perl version for the pg_upgrade tests is to achieve equality of dumps for most cross-versions cases. If so this is the significant improvement as previously in test.sh resulted dumps retained unequal and the user was asked to eyeball them manually during cross upgrades between different major versions. So, the backport of the perl tests also seems preferable to me. I don't really agree with that. These TAP tests are really new development, and it took a few tries to get them completely right (well, as much right as it holds for HEAD). If we were to backport any of this, there is a risk of introducing a bug in what we do with any of that, potentially hiding a issue critical related to pg_upgrade. That's not worth taking a risk for. Saying that, I agree that more needs to be done, but I would limit that only to HEAD and let it mature more into the tree in an incremental fashion. -- I have withdrawn the patch with the backport, but then the question is whether we will make fixes in older test.sh tests seems to be remains open. Will we fix it? Justin is not sure if anyone needs this: https://www.postgresql.org/message-id/67b6b447-e9cb-ebde-4a6b-127aea7ca268%40inbox.ru Also found that the test from older versions fails in the current master. Proposed a fix in a new thread: https://www.postgresql.org/message-id/49f389ba-95ce-8a9b-09ae-f60650c0e7c7%40inbox.ru Would be glad to any remarks. With the best wishes, -- Anton A. Melnikov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
Re: [PATCH] Backport perl tests for pg_upgrade from 322becb60
On Mon, Dec 19, 2022 at 04:16:53AM +0300, Anton A. Melnikov wrote: > I have withdrawn the patch with the backport, but then the question is > whether we > will make fixes in older test.sh tests seems to be remains open. > Will we fix it? Justin is not sure if anyone needs this: > https://www.postgresql.org/message-id/67b6b447-e9cb-ebde-4a6b-127aea7ca268%40inbox.ru This introduces an extra maintenance cost over the existing things in the stable branches. > Also found that the test from older versions fails in the current master. > Proposed a fix in a new thread: > https://www.postgresql.org/message-id/49f389ba-95ce-8a9b-09ae-f60650c0e7c7%40inbox.ru Thanks. Yes, this is the change of aclitem from 32b to 64b, which is something that needs some tweaks. So let's fix this one. -- Michael signature.asc Description: PGP signature
Re: [PATCH] Backport perl tests for pg_upgrade from 322becb60
Add backport to REL_14_STABLE. Unlike to the 13th version's one there are still some differences in the final dumps, eg during upgrade test 12->14. The similar differences present during upgrade test 12->master. Achieving zero dump diffs needs additional work, now in progress. With best regards, -- Anton A. Melnikov Postgres Professional: http://www.postgrespro.com The Russian Postgres Companydiff --git a/src/bin/pg_upgrade/.gitignore b/src/bin/pg_upgrade/.gitignore index 2d3bfeaa50..05200a09f1 100644 --- a/src/bin/pg_upgrade/.gitignore +++ b/src/bin/pg_upgrade/.gitignore @@ -1,9 +1,4 @@ /pg_upgrade # Generated by test suite -/pg_upgrade_internal.log -/delete_old_cluster.sh -/delete_old_cluster.bat -/reindex_hash.sql -/loadable_libraries.txt /log/ /tmp_check/ diff --git a/src/bin/pg_upgrade/Makefile b/src/bin/pg_upgrade/Makefile index 44d06be5a6..105199f182 100644 --- a/src/bin/pg_upgrade/Makefile +++ b/src/bin/pg_upgrade/Makefile @@ -28,6 +28,10 @@ OBJS = \ override CPPFLAGS := -DDLSUFFIX=\"$(DLSUFFIX)\" -I$(srcdir) -I$(libpq_srcdir) $(CPPFLAGS) LDFLAGS_INTERNAL += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport) +# required for 002_pg_upgrade.pl +REGRESS_SHLIB=$(abs_top_builddir)/src/test/regress/regress$(DLSUFFIX) +export REGRESS_SHLIB + all: pg_upgrade pg_upgrade: $(OBJS) | submake-libpq submake-libpgport submake-libpgfeutils @@ -44,22 +48,10 @@ uninstall: clean distclean maintainer-clean: rm -f pg_upgrade$(X) $(OBJS) - rm -rf delete_old_cluster.sh log/ tmp_check/ \ - loadable_libraries.txt reindex_hash.sql \ - pg_upgrade_dump_globals.sql \ - pg_upgrade_dump_*.custom pg_upgrade_*.log - -# When $(MAKE) is present, make automatically infers that this is a -# recursive make. which is not actually what we want here, as that -# e.g. prevents output synchronization from working (as make thinks -# that the subsidiary make knows how to deal with that itself, but -# we're invoking a shell script that doesn't know). Referencing -# $(MAKE) indirectly avoids that behaviour. -# See https://www.gnu.org/software/make/manual/html_node/MAKE-Variable.html#MAKE-Variable -NOTSUBMAKEMAKE=$(MAKE) + rm -rf log/ tmp_check/ -check: test.sh all temp-install - MAKE=$(NOTSUBMAKEMAKE) $(with_temp_install) bindir=$(abs_top_builddir)/tmp_install/$(bindir) EXTRA_REGRESS_OPTS="$(EXTRA_REGRESS_OPTS)" $(SHELL) $< +check: + $(prove_check) -# installcheck is not supported because there's no meaningful way to test -# pg_upgrade against a single already-running server +installcheck: + $(prove_installcheck) diff --git a/src/bin/pg_upgrade/TESTING b/src/bin/pg_upgrade/TESTING index e69874b42d..200ce9d92b 100644 --- a/src/bin/pg_upgrade/TESTING +++ b/src/bin/pg_upgrade/TESTING @@ -2,25 +2,22 @@ THE SHORT VERSION - On non-Windows machines, you can execute the testing process -described below by running +described below by running the following command in this directory: make check -in this directory. This will run the shell script test.sh, performing -an upgrade from the version in this source tree to a new instance of -the same version. -To test an upgrade from a different version, you must have a built -source tree for the old version as well as this version, and you -must have done "make install" for both versions. Then do: +This will run the TAP tests to run pg_upgrade, performing an upgrade +from the version in this source tree to a new instance of the same +version. -export oldsrc=...somewhere/postgresql (old version's source tree) -export oldbindir=...otherversion/bin (old version's installed bin dir) -export bindir=...thisversion/bin (this version's installed bin dir) -export libdir=...thisversion/lib (this version's installed lib dir) -sh test.sh - -In this case, you will have to manually eyeball the resulting dump -diff for version-specific differences, as explained below. +Testing an upgrade from a different version requires a dump to set up +the contents of this instance, with its set of binaries. The following +variables are available to control the test (see DETAILS below about +the creation of the dump): +export olddump=...somewhere/dump.sql (old version's dump) +export oldinstall=...otherversion/ (old version's install base path) +Finally, the tests can be done by running + make check DETAILS --- @@ -29,61 +26,22 @@ The most effective way to test pg_upgrade, aside from testing on user data, is by upgrading the PostgreSQL regression database. This testing process first requires the creation of a valid regression -database dump. Such files contain most database features and are -specific to each major version of Postgres. +database dump that can be then used for $olddump. Such files contain +most database features and are specific to each major version of Postgres. -Here are the steps needed to create a regression database dump file: +He
Re: [PATCH] Backport perl tests for pg_upgrade from 322becb60
On Mon, Aug 01, 2022 at 01:02:21AM +0300, Anton A. Melnikov wrote: > As far as i understand from this thread: > https://www.postgresql.org/message-id/flat/Yox1ME99GhAemMq1%40paquier.xyz, > the aim of the perl version for the pg_upgrade tests is to achieve equality > of dumps for most cross-versions cases. > If so this is the significant improvement as previously in test.sh resulted > dumps retained unequal and the user > was asked to eyeball them manually during cross upgrades between different > major versions. > So, the backport of the perl tests also seems preferable to me. I don't really agree with that. These TAP tests are really new development, and it took a few tries to get them completely right (well, as much right as it holds for HEAD). If we were to backport any of this, there is a risk of introducing a bug in what we do with any of that, potentially hiding a issue critical related to pg_upgrade. That's not worth taking a risk for. Saying that, I agree that more needs to be done, but I would limit that only to HEAD and let it mature more into the tree in an incremental fashion. -- Michael signature.asc Description: PGP signature
