pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedFrom: Justin Pryzby <pryzby@telsasoft.com>
To: Andres Freund <andres@anarazel.de>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Andrew Dunstan <andrew@dunslane.net>
Cc: pgsql-hackers@postgresql.org, Thomas Munro <thomas.munro@gmail.com>
Cc: Melanie Plageman <melanieplageman@gmail.com>
Cc: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Cc: Daniel Gustafsson <daniel@yesql.se>
Subject: Re: Adding CI to our tree
Date: Sun, 13 Feb 2022 15:42:13 -0600
Message-ID: <20220213214213.GS31460@telsasoft.com> (raw)
In-Reply-To: <20220212222625.aph3ft466ntifrpi@alap3.anarazel.de>
References: <20220117192510.txue5mihjaxlngep@alap3.anarazel.de>
<85428.1642447853@sss.pgh.pa.us>
<20220117201619.3ltudwhgk2krmoki@alap3.anarazel.de>
<20220118210847.GC23027@telsasoft.com>
<20220203035827.GG23027@telsasoft.com>
<20220203195718.smqo5xg4ygp5qktq@alap3.anarazel.de>
<20220204050403.GL23027@telsasoft.com>
<20220206032339.tuyo534rfbvu4mbh@alap3.anarazel.de>
<20220212220640.GL31460@telsasoft.com>
<20220212222625.aph3ft466ntifrpi@alap3.anarazel.de>
On Sat, Feb 12, 2022 at 02:26:25PM -0800, Andres Freund wrote:
> On 2022-02-12 16:06:40 -0600, Justin Pryzby wrote:
> > I had some success with that, but it doesn't seem to be significantly faster -
> > it looks a lot like the tests are not actually running in parallel.
Note that the total test time is close to the sum of the individual test times.
But I think that may be an artifact of how prove is showing/attributing times
to each test (which, if so, is misleading).
> Note that prove unfortunately serializes the test output to be in the order it
> started them, even when tests run concurrently. Extremely unhelpful, but ...
Are you sure ? When I run it locally, I see:
rm -fr src/test/recovery/tmp_check ; time PERL5LIB=`pwd`/src/test/perl TESTDIR=`pwd`/src/test/recovery PATH=`pwd`/tmp_install/usr/local/pgsql/bin:$PATH PG_REGRESS=`pwd`/src/test/regress/pg_regress REGRESS_SHLIB=`pwd`/src/test/regress/regress.so prove --time -j4 --ext '*.pl' `find src -name t`
...
[15:34:48] src/bin/scripts/t/101_vacuumdb_all.pl ....................... ok 104 ms ( 0.00 usr 0.00 sys + 2.35 cusr 0.47 csys = 2.82 CPU)
[15:34:49] src/bin/scripts/t/090_reindexdb.pl .......................... ok 8894 ms ( 0.06 usr 0.01 sys + 14.45 cusr 3.38 csys = 17.90 CPU)
[15:34:50] src/bin/pg_config/t/001_pg_config.pl ........................ ok 79 ms ( 0.00 usr 0.01 sys + 0.23 cusr 0.04 csys = 0.28 CPU)
[15:34:50] src/bin/pg_waldump/t/001_basic.pl ........................... ok 35 ms ( 0.00 usr 0.00 sys + 0.26 cusr 0.02 csys = 0.28 CPU)
[15:34:51] src/bin/pg_test_fsync/t/001_basic.pl ........................ ok 100 ms ( 0.01 usr 0.00 sys + 0.24 cusr 0.04 csys = 0.29 CPU)
[15:34:51] src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl ........ ok 177 ms ( 0.02 usr 0.00 sys + 0.26 cusr 0.03 csys = 0.31 CPU)
[15:34:55] src/bin/scripts/t/100_vacuumdb.pl ........................... ok 11267 ms ( 0.12 usr 0.04 sys + 13.47 cusr 3.20 csys = 16.83 CPU)
[15:34:57] src/bin/scripts/t/102_vacuumdb_stages.pl .................... ok 5802 ms ( 0.06 usr 0.01 sys + 7.70 cusr 1.37 csys = 9.14 CPU)
...
=> scripts/ stuff, followed by other stuff, followed by more, slower, scripts/ stuff.
But I never saw that in cirrus.
> Isn't this kind of a good test time? I thought earlier your alltaptests target
> took a good bit longer?
The original alltaptests runs in 16m 21s.
https://cirrus-ci.com/task/6679061752709120
2 weeks ago, it was ~14min with your patch to cache initdb.
https://cirrus-ci.com/task/5439320633901056
As I recall, that didn't seem to improve runtime when combined with my parallel
patch.
> One nice bit is that the output is a *lot* easier to read.
>
> You could try improving the total time by having prove remember slow tests and
> use that file to run the slowest tests first next time. --state slow,save or
> such I believe. Of course we'd have to save that state file...
In a test, this hurt rather than helped (13m 42s).
https://cirrus-ci.com/task/6359167186239488
I'm not surprised - it makes sense to run 10 fast tests at once, but usually
doesn't make sense to run 10 slow tests tests at once (at least a couple of
which are doing something intensive). It was faster (12m16s) to do it
backwards (fastest tests first).
https://cirrus-ci.com/task/5745115443494912
BTW, does it make sense to remove test_regress_parallel_script ? The
pg_upgrade run would do the same things, no ? If so, it might make sense to
run that first. OTOH, you suggested to run the upgrade tests with checksums
enabled, which seems like a good idea.
Note that in the attached patches, I changed the msvc "warnings" to use "tee".
I don't know how to fix the pipeline test in a less hacky way...
You said the docs build should be a separate task, but then said that it'd be
okay to remove the dependency. So I did it both ways. There's currently some
duplication between the docs patch and code coverage patch.
--
Justin
Attachments:
[text/x-diff] 0001-cirrus-include-hints-how-to-install-OS-packages.patch (2.0K, ../20220213214213.GS31460@telsasoft.com/2-0001-cirrus-include-hints-how-to-install-OS-packages.patch)
download | inline diff:
From 05c24a1e679e5ae0dd0cb6504d397f77c8b1fc5c Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Mon, 17 Jan 2022 00:53:04 -0600
Subject: [PATCH 1/8] cirrus: include hints how to install OS packages..
This is useful for patches during development, but once a features is merged,
new libraries should be added to the OS image files, rather than installed
during every CI run forever into the future.
---
.cirrus.yml | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index dd96a97efe5..eda8ac9596c 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -73,10 +73,11 @@ task:
chown -R postgres:postgres .
mkdir -p ${CCACHE_DIR}
chown -R postgres:postgres ${CCACHE_DIR}
- setup_cores_script: |
+ setup_os_script: |
mkdir -m 770 /tmp/cores
chown root:postgres /tmp/cores
sysctl kern.corefile='/tmp/cores/%N.%P.core'
+ #pkg install -y ...
# NB: Intentionally build without --with-llvm. The freebsd image size is
# already large enough to make VM startup slow, and even without llvm
@@ -180,10 +181,12 @@ task:
chown -R postgres:postgres ${CCACHE_DIR}
echo '* - memlock 134217728' > /etc/security/limits.d/postgres.conf
su postgres -c "ulimit -l -H && ulimit -l -S"
- setup_cores_script: |
+ setup_os_script: |
mkdir -m 770 /tmp/cores
chown root:postgres /tmp/cores
sysctl kernel.core_pattern='/tmp/cores/%e-%s-%p.core'
+ #apt-get update
+ #apt-get -y install ...
configure_script: |
su postgres <<-EOF
@@ -237,7 +240,7 @@ task:
ulimit -a -H && ulimit -a -S
export
- setup_cores_script:
+ setup_os_script:
- mkdir ${HOME}/cores
- sudo sysctl kern.corefile="${HOME}/cores/core.%P"
@@ -384,6 +387,9 @@ task:
powershell -Command get-psdrive -psprovider filesystem
set
+ setup_os_script: |
+ REM choco install -y ...
+
configure_script:
# copy errors out when using forward slashes
- copy src\tools\ci\windows_build_config.pl src\tools\msvc\config.pl
--
2.17.1
[text/x-diff] 0002-cirrus-windows-add-compiler_warnings_script.patch (1.1K, ../20220213214213.GS31460@telsasoft.com/3-0002-cirrus-windows-add-compiler_warnings_script.patch)
download | inline diff:
From 68776b7e6178c2582715261b7526a520f39063ad Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sat, 12 Feb 2022 18:59:50 -0600
Subject: [PATCH 2/8] cirrus/windows: add compiler_warnings_script
ci-os-only: windows
https://cirrus-ci.com/task/6316260295180288
---
.cirrus.yml | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index eda8ac9596c..2d023a31d2d 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -397,7 +397,7 @@ task:
- perl src/tools/msvc/mkvcbuild.pl
build_script:
- vcvarsall x64
- - msbuild %MSBFLAGS% pgsql.sln
+ - msbuild %MSBFLAGS% pgsql.sln |tee build.log
tempinstall_script:
# Installation on windows currently only completely works from src/tools/msvc
- cd src/tools/msvc && perl install.pl %CIRRUS_WORKING_DIR%/tmp_install
@@ -440,6 +440,11 @@ task:
cd src/tools/msvc
%T_C% perl vcregress.pl ecpgcheck
+ always:
+ compiler_warnings_script: |
+ findstr "warning " build.log && exit /b 1
+ exit /b 0
+
on_failure:
<<: *on_failure
crashlog_artifacts:
--
2.17.1
[text/x-diff] 0003-cirrus-upload-changed-html-docs-as-artifacts.patch (2.6K, ../20220213214213.GS31460@telsasoft.com/4-0003-cirrus-upload-changed-html-docs-as-artifacts.patch)
download | inline diff:
From 977f8a1456b18744ac552a3f7f92f2840f625e08 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sat, 15 Jan 2022 11:27:28 -0600
Subject: [PATCH 3/8] cirrus: upload changed html docs as artifacts
Always run doc build; to allow them to be shown in cfbot, they should not be
skipped if the linux build fails.
This could be done on the client side (cfbot). One advantage of doing it here
is that fewer docs are uploaded - many patches won't upload docs at all.
https://cirrus-ci.com/task/5396696388599808
ci-os-only: linux
---
.cirrus.yml | 43 +++++++++++++++++++++++++++++++------------
1 file changed, 31 insertions(+), 12 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index 2d023a31d2d..39021804cfa 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -455,10 +455,6 @@ task:
task:
name: CompilerWarnings
- # To limit unnecessary work only run this once the normal linux test succeeds
- depends_on:
- - Linux - Debian Bullseye
-
env:
CPUS: 4
BUILD_JOBS: 4
@@ -487,6 +483,13 @@ task:
clang -v
export
+ git remote -v
+ git branch -a
+ git remote add postgres https://github.com/postgres/postgres
+ time git fetch -v postgres master
+ git log -1 postgres/master
+ git diff --name-only postgres/master..
+
ccache_cache:
folder: $CCACHE_DIR
@@ -558,16 +561,32 @@ task:
###
# Verify docs can be built
###
- # XXX: Only do this if there have been changes in doc/ since last build
always:
docs_build_script: |
- time ./configure \
- --cache gcc.cache \
- CC="ccache gcc" \
- CXX="ccache g++" \
- CLANG="ccache clang"
- make -s -j${BUILD_JOBS} clean
- time make -s -j${BUILD_JOBS} -C doc
+ # Do nothing if the patch doesn't update doc:
+ git diff --name-only --cherry-pick --exit-code postgres/master... doc && exit
+
+ # Exercise HTML and other docs:
+ ./configure >/dev/null
+ make -s -C doc
+ cp -a doc new-docs
+
+ # Build HTML docs from the parent commit
+ git checkout postgres/master -- doc
+ make -s -C doc clean
+ make -s -C doc html
+ cp -a doc old-docs
+
+ # Copy HTML which differ into html_docs
+ mkdir html_docs
+ cp new-docs/src/sgml/html/*.css new-docs/src/sgml/html/*.svg html_docs/
+ for f in `git diff --no-index --name-only old-docs/src/sgml/html new-docs/src/sgml/html`
+ do
+ cp $f html_docs/
+ done
+
+ html_docs_artifacts:
+ paths: ['html_docs/**/*.html', 'html_docs/**/*.png', 'html_docs/**/*.css']
always:
upload_caches: ccache
--
2.17.1
[text/x-diff] 0004-s-build-docs-as-a-separate-task.patch (3.6K, ../20220213214213.GS31460@telsasoft.com/5-0004-s-build-docs-as-a-separate-task.patch)
download | inline diff:
From a09f7bba347e84f74f0a424043161f8f7874ba92 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sun, 13 Feb 2022 03:28:00 -0600
Subject: [PATCH 4/8] s!build docs as a separate task..
I believe this'll automatically show up as a separate "column" on the cfbot
page.
---
.cirrus.yml | 95 ++++++++++++++++++++++++++++++++---------------------
1 file changed, 58 insertions(+), 37 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index 39021804cfa..9a45a876904 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -455,6 +455,10 @@ task:
task:
name: CompilerWarnings
+ # To limit unnecessary work only run this once the normal linux test succeeds
+ depends_on:
+ - Linux - Debian Bullseye
+
env:
CPUS: 4
BUILD_JOBS: 4
@@ -483,13 +487,6 @@ task:
clang -v
export
- git remote -v
- git branch -a
- git remote add postgres https://github.com/postgres/postgres
- time git fetch -v postgres master
- git log -1 postgres/master
- git diff --name-only postgres/master..
-
ccache_cache:
folder: $CCACHE_DIR
@@ -558,35 +555,59 @@ task:
make -s -j${BUILD_JOBS} clean
time make -s -j${BUILD_JOBS} world-bin
- ###
- # Verify docs can be built
- ###
- always:
- docs_build_script: |
- # Do nothing if the patch doesn't update doc:
- git diff --name-only --cherry-pick --exit-code postgres/master... doc && exit
-
- # Exercise HTML and other docs:
- ./configure >/dev/null
- make -s -C doc
- cp -a doc new-docs
-
- # Build HTML docs from the parent commit
- git checkout postgres/master -- doc
- make -s -C doc clean
- make -s -C doc html
- cp -a doc old-docs
-
- # Copy HTML which differ into html_docs
- mkdir html_docs
- cp new-docs/src/sgml/html/*.css new-docs/src/sgml/html/*.svg html_docs/
- for f in `git diff --no-index --name-only old-docs/src/sgml/html new-docs/src/sgml/html`
- do
- cp $f html_docs/
- done
-
- html_docs_artifacts:
- paths: ['html_docs/**/*.html', 'html_docs/**/*.png', 'html_docs/**/*.css']
-
always:
upload_caches: ccache
+
+
+# Verify docs can be built, and upload changed docs as artifacts
+task:
+ name: HTML docs
+
+ env:
+ CPUS: 1
+
+ only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*(docs|html).*'
+
+ container:
+ image: $CONTAINER_REPO/linux_debian_bullseye_ci:latest
+ cpu: $CPUS
+
+ sysinfo_script: |
+ id
+ uname -a
+ cat /proc/cmdline
+ ulimit -a -H && ulimit -a -S
+ export
+
+ git remote -v
+ git branch -a
+ git remote add postgres https://github.com/postgres/postgres
+ time git fetch -v postgres master
+ git log -1 postgres/master
+ git diff --name-only postgres/master..
+
+ docs_build_script: |
+ # Do nothing if the patch doesn't update doc:
+ git diff --name-only --cherry-pick --exit-code postgres/master... doc && exit
+
+ # Exercise HTML and other docs:
+ ./configure >/dev/null
+ make -s -C doc
+ cp -a doc new-docs
+
+ # Build HTML docs from the parent commit
+ git checkout postgres/master -- doc
+ make -s -C doc clean
+ make -s -C doc html
+ cp -a doc old-docs
+
+ # Copy HTML which differ into html_docs
+ mkdir html_docs
+ cp new-docs/src/sgml/html/*.css new-docs/src/sgml/html/*.svg html_docs/
+ for f in `git diff --no-index --name-only old-docs/src/sgml/html new-docs/src/sgml/html`
+ do
+ cp $f html_docs/
+ done
+
+ html_docs_artifacts:
+ paths: ['html_docs/**/*.html', 'html_docs/**/*.png', 'html_docs/**/*.css']
--
2.17.1
[text/x-diff] 0005-vcregress-ci-test-modules-contrib-with-NO_INSTALLCHE.patch (7.8K, ../20220213214213.GS31460@telsasoft.com/6-0005-vcregress-ci-test-modules-contrib-with-NO_INSTALLCHE.patch)
download | inline diff:
From 4b4e30eb8d78b3730bfbc7bff8194e79c203d8d4 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sun, 9 Jan 2022 18:25:02 -0600
Subject: [PATCH 5/8] vcregress/ci: test modules/contrib with NO_INSTALLCHECK=1
---
.cirrus.yml | 4 +-
contrib/basic_archive/Makefile | 2 +-
contrib/pg_stat_statements/Makefile | 2 +-
contrib/test_decoding/Makefile | 2 +-
src/test/modules/snapshot_too_old/Makefile | 2 +-
src/test/modules/worker_spi/Makefile | 2 +-
src/tools/msvc/vcregress.pl | 46 +++++++++++++++++++---
7 files changed, 48 insertions(+), 12 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index 9a45a876904..666901fe0c7 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -415,9 +415,9 @@ task:
test_isolation_script: |
%T_C% perl src/tools/msvc/vcregress.pl isolationcheck
test_modules_script: |
- %T_C% perl src/tools/msvc/vcregress.pl modulescheck
+ %T_C% perl src/tools/msvc/vcregress.pl modulescheck install
test_contrib_script: |
- %T_C% perl src/tools/msvc/vcregress.pl contribcheck
+ %T_C% perl src/tools/msvc/vcregress.pl contribcheck install
stop_script: |
tmp_install\bin\pg_ctl.exe stop -D tmp_check/db -l tmp_check/postmaster.log
test_ssl_script: |
diff --git a/contrib/basic_archive/Makefile b/contrib/basic_archive/Makefile
index 14d036e1c42..246358973fe 100644
--- a/contrib/basic_archive/Makefile
+++ b/contrib/basic_archive/Makefile
@@ -4,7 +4,7 @@ MODULES = basic_archive
PGFILEDESC = "basic_archive - basic archive module"
REGRESS = basic_archive
-REGRESS_OPTS = --temp-config $(top_srcdir)/contrib/basic_archive/basic_archive.conf
+REGRESS_OPTS = --temp-config=$(top_srcdir)/contrib/basic_archive/basic_archive.conf
NO_INSTALLCHECK = 1
diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile
index 7fabd96f38d..d732e1ade73 100644
--- a/contrib/pg_stat_statements/Makefile
+++ b/contrib/pg_stat_statements/Makefile
@@ -15,7 +15,7 @@ PGFILEDESC = "pg_stat_statements - execution statistics of SQL statements"
LDFLAGS_SL += $(filter -lm, $(LIBS))
-REGRESS_OPTS = --temp-config $(top_srcdir)/contrib/pg_stat_statements/pg_stat_statements.conf
+REGRESS_OPTS = --temp-config=$(top_srcdir)/contrib/pg_stat_statements/pg_stat_statements.conf
REGRESS = pg_stat_statements oldextversions
# Disabled because these tests require "shared_preload_libraries=pg_stat_statements",
# which typical installcheck users do not have (e.g. buildfarm clients).
diff --git a/contrib/test_decoding/Makefile b/contrib/test_decoding/Makefile
index 56ddc3abaeb..0b69ee5c388 100644
--- a/contrib/test_decoding/Makefile
+++ b/contrib/test_decoding/Makefile
@@ -11,7 +11,7 @@ ISOLATION = mxact delayed_startup ondisk_startup concurrent_ddl_dml \
oldest_xmin snapshot_transfer subxact_without_top concurrent_stream \
twophase_snapshot
-REGRESS_OPTS = --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf
+REGRESS_OPTS = --temp-config=$(top_srcdir)/contrib/test_decoding/logical.conf
ISOLATION_OPTS = --temp-config $(top_srcdir)/contrib/test_decoding/logical.conf
# Disabled because these tests require "wal_level=logical", which
diff --git a/src/test/modules/snapshot_too_old/Makefile b/src/test/modules/snapshot_too_old/Makefile
index dfb4537f63c..752a0039fdc 100644
--- a/src/test/modules/snapshot_too_old/Makefile
+++ b/src/test/modules/snapshot_too_old/Makefile
@@ -5,7 +5,7 @@
EXTRA_CLEAN = $(pg_regress_clean_files)
ISOLATION = sto_using_cursor sto_using_select sto_using_hash_index
-ISOLATION_OPTS = --temp-config $(top_srcdir)/src/test/modules/snapshot_too_old/sto.conf
+ISOLATION_OPTS = --temp-config=$(top_srcdir)/src/test/modules/snapshot_too_old/sto.conf
# Disabled because these tests require "old_snapshot_threshold" >= 0, which
# typical installcheck users do not have (e.g. buildfarm clients).
diff --git a/src/test/modules/worker_spi/Makefile b/src/test/modules/worker_spi/Makefile
index cbf9b2e37fd..d9f7d9bab6d 100644
--- a/src/test/modules/worker_spi/Makefile
+++ b/src/test/modules/worker_spi/Makefile
@@ -9,7 +9,7 @@ PGFILEDESC = "worker_spi - background worker example"
REGRESS = worker_spi
# enable our module in shared_preload_libraries for dynamic bgworkers
-REGRESS_OPTS = --temp-config $(top_srcdir)/src/test/modules/worker_spi/dynamic.conf
+REGRESS_OPTS = --temp-config=$(top_srcdir)/src/test/modules/worker_spi/dynamic.conf
# Disable installcheck to ensure we cover dynamic bgworkers.
NO_INSTALLCHECK = 1
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index ddce4680a94..0dfa7e8dad5 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -441,6 +441,7 @@ sub plcheck
sub subdircheck
{
my $module = shift;
+ my $installcheck = shift || 1;
if ( !-d "$module/sql"
|| !-d "$module/expected"
@@ -450,7 +451,7 @@ sub subdircheck
}
chdir $module;
- my @tests = fetchTests();
+ my @tests = fetchTests($installcheck);
# Leave if no tests are listed in the module.
if (scalar @tests == 0)
@@ -460,6 +461,7 @@ sub subdircheck
}
my @opts = fetchRegressOpts();
+ push @opts, "--temp-instance=tmp_check" if $installcheck == -1;
# Special processing for python transform modules, see their respective
# Makefiles for more details regarding Python-version specific
@@ -487,7 +489,7 @@ sub subdircheck
print "Checking $module\n";
my @args = (
"$topdir/$Config/pg_regress/pg_regress",
- "--bindir=${topdir}/${Config}/psql",
+ "--bindir=$tmp_installdir/bin",
"--dbname=contrib_regression", @opts, @tests);
print join(' ', @args), "\n";
system(@args);
@@ -497,6 +499,8 @@ sub subdircheck
sub contribcheck
{
+ my $mode = shift || '';
+
chdir "../../../contrib";
my $mstat = 0;
foreach my $module (glob("*"))
@@ -514,12 +518,25 @@ sub contribcheck
my $status = $? >> 8;
$mstat ||= $status;
}
+
+ # As above, but creates new DB instance for each module. For CI.
+ if ($mode eq "install")
+ {
+ foreach my $module (glob("*"))
+ {
+ subdircheck("$module", -1);
+ $mstat ||= $? >> 8;
+ }
+ }
+
exit $mstat if $mstat;
return;
}
sub modulescheck
{
+ my $mode = shift || '';
+
chdir "../../../src/test/modules";
my $mstat = 0;
foreach my $module (glob("*"))
@@ -528,6 +545,17 @@ sub modulescheck
my $status = $? >> 8;
$mstat ||= $status;
}
+
+ # As above, but creates new DB instance for each module. For CI.
+ if ($mode eq "install")
+ {
+ foreach my $module (glob("*"))
+ {
+ subdircheck("$module", -1);
+ $mstat ||= $? >> 8;
+ }
+ }
+
exit $mstat if $mstat;
return;
}
@@ -699,6 +727,7 @@ sub fetchRegressOpts
# option starting with "--".
@opts = grep { !/\$\(/ && /^--/ }
map { (my $x = $_) =~ s/\Q$(top_builddir)\E/\"$topdir\"/; $x; }
+ map { (my $x = $_) =~ s/\Q$(top_srcdir)\E/\"$topdir\"/; $x; }
split(/\s+/, $1);
}
if ($m =~ /^\s*ENCODING\s*=\s*(\S+)/m)
@@ -724,14 +753,19 @@ sub fetchTests
my $m = <$handle>;
close($handle);
my $t = "";
+ my $installcheck = shift || 1;
$m =~ s{\\\r?\n}{}g;
- # A module specifying NO_INSTALLCHECK does not support installcheck,
- # so bypass its run by returning an empty set of tests.
if ($m =~ /^\s*NO_INSTALLCHECK\s*=\s*\S+/m)
{
- return ();
+ # Skip modules marked installcheck unless running installcheck tests.
+ return () if $installcheck == 1;
+ }
+ else
+ {
+ # Skip modules not marked installcheck if running installcheck tests.
+ return () if $installcheck == -1;
}
if ($m =~ /^REGRESS\s*=\s*(.*)$/gm)
@@ -797,6 +831,8 @@ sub usage
"\nOptions for <arg>: (used by check and installcheck)\n",
" serial serial mode\n",
" parallel parallel mode\n",
+ "\nOptions for <arg>: (used by contribcheck and modulescheck)\n",
+ " install also run tests which require a new instance\n",
"\nOption for <arg>: for taptest\n",
" TEST_DIR (required) directory where tests reside\n";
exit(1);
--
2.17.1
[text/x-diff] 0006-wip-cirrus-code-coverage.patch (2.6K, ../20220213214213.GS31460@telsasoft.com/7-0006-wip-cirrus-code-coverage.patch)
download | inline diff:
From c82966ba7d9dfe8a801670c1a07bc9204dcd89ea Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Mon, 17 Jan 2022 00:54:28 -0600
Subject: [PATCH 6/8] wip: cirrus: code coverage
XXX: lcov should be installed in the OS image
---
.cirrus.yml | 34 ++++++++++++++++++++++++++++++++--
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index 666901fe0c7..5efe9733e85 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -174,6 +174,15 @@ task:
cat /proc/cmdline
ulimit -a -H && ulimit -a -S
export
+ git remote -v
+ git branch -a
+ git remote add postgres https://github.com/postgres/postgres
+ time git fetch -v postgres master
+ #git branch postgresmaster FETCH_HEAD
+ git log -1 postgres/master
+ git branch parent `git merge-base postgres/master HEAD`
+ #git diff --name-only --cherry-pick postgres/master...
+ git diff --name-only parent..
create_user_script: |
useradd -m postgres
chown -R postgres:postgres .
@@ -185,13 +194,14 @@ task:
mkdir -m 770 /tmp/cores
chown root:postgres /tmp/cores
sysctl kernel.core_pattern='/tmp/cores/%e-%s-%p.core'
- #apt-get update
- #apt-get -y install ...
+ apt-get update
+ apt-get -y install lcov
configure_script: |
su postgres <<-EOF
./configure \
--enable-cassert --enable-debug --enable-tap-tests \
+ --enable-coverage \
--enable-nls \
\
${LINUX_CONFIGURE_FEATURES} \
@@ -211,6 +221,26 @@ task:
make -s ${CHECK} ${CHECKFLAGS} -j${TEST_JOBS}
EOF
+ generate_coverage_report_script: |
+ mkdir coverage
+ # Coverage only for changed files:
+ for f in `git diff --name-only --cherry-pick postgres/master... '*.c'`;
+ do
+ lcov --quiet --capture --directory "$f"
+ done >coverage/coverage.gcov
+
+ # Coverage for all files:
+ time lcov --quiet --capture --directory . >coverage/coverage-all.gcov
+
+ time genhtml coverage/coverage.gcov --output-directory coverage --show-details --legend --quiet
+ time genhtml coverage/coverage-all.gcov --output-directory coverage-all --show-details --legend --quiet
+ cp coverage/index.html coverage/00-index.html
+ cp coverage-all/index.html coverage-all/00-index.html
+ #git diff --name-only --cherry-pick postgres/master... '*.c' |xargs -r gcov --demangled-names --
+
+ coverage_artifacts:
+ paths: ['coverage/**/*.html', 'coverage/**/*.png', 'coverage/**/*.gcov', 'coverage/**/*.css' ]
+
on_failure:
<<: *on_failure
cores_script: src/tools/ci/cores_backtrace.sh linux /tmp/cores
--
2.17.1
[text/x-diff] 0007-vcsregress-add-alltaptests.patch (4.2K, ../20220213214213.GS31460@telsasoft.com/8-0007-vcsregress-add-alltaptests.patch)
download | inline diff:
From d91ef7675b0aaede7f9ac2b4b717e7769883d155 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sat, 12 Feb 2022 20:52:08 -0600
Subject: [PATCH 7/8] vcsregress: add alltaptests
In passing, document taptest PROVE_FLAGS
See also:
d835dd6685246f0737ca42ab68242210681bb220
13d856e177e69083f543d6383eeda9e12ce3c55c
fed6df486dca1b9e53d3f560031b9a236c99f4bb
---
.cirrus.yml | 10 ++-------
src/tools/msvc/vcregress.pl | 44 +++++++++++++++++++++++++++++++++----
2 files changed, 42 insertions(+), 12 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index 5efe9733e85..fac26d63f54 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -453,14 +453,8 @@ task:
test_ssl_script: |
set with_ssl=openssl
%T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/ssl/
- test_subscription_script: |
- %T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/subscription/
- test_authentication_script: |
- %T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/authentication/
- test_recovery_script: |
- %T_C% perl src/tools/msvc/vcregress.pl recoverycheck
- test_bin_script: |
- %T_C% perl src/tools/msvc/vcregress.pl bincheck
+ test_tap_script: |
+ %T_C% perl src/tools/msvc/vcregress.pl alltaptests
test_pg_upgrade_script: |
%T_C% perl src/tools/msvc/vcregress.pl upgradecheck
test_ecpg_script: |
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 0dfa7e8dad5..4572db8eca4 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -48,7 +48,7 @@ if (-e "src/tools/msvc/buildenv.pl")
my $what = shift || "";
if ($what =~
- /^(check|installcheck|plcheck|contribcheck|modulescheck|ecpgcheck|isolationcheck|upgradecheck|bincheck|recoverycheck|taptest)$/i
+ /^(check|installcheck|plcheck|contribcheck|modulescheck|ecpgcheck|isolationcheck|upgradecheck|bincheck|recoverycheck|taptest|alltaptests)$/i
)
{
$what = uc $what;
@@ -106,6 +106,7 @@ my %command = (
BINCHECK => \&bincheck,
RECOVERYCHECK => \&recoverycheck,
UPGRADECHECK => \&upgradecheck,
+ ALLTAPTESTS => \&alltaptests,
TAPTEST => \&taptest,);
my $proc = $command{$what};
@@ -265,6 +266,9 @@ sub tap_check
# add the module build dir as the second element in the PATH
$ENV{PATH} =~ s!;!;$topdir/$Config/$module;!;
+ print "============================================================\n";
+ print "Checking @args\n";
+
rmtree('tmp_check');
system(@args);
my $status = $? >> 8;
@@ -277,8 +281,7 @@ sub bincheck
my $mstat = 0;
- # Find out all the existing TAP tests by looking for t/ directories
- # in the tree.
+ # Find the TAP tests by looking for t/ directories
my @bin_dirs = glob("$topdir/src/bin/*");
# Process each test
@@ -292,6 +295,37 @@ sub bincheck
return;
}
+sub alltaptests
+{
+ InstallTemp();
+
+ my $mstat = 0;
+
+ # Find out all the existing TAP tests by looking for t/ directories
+ # in the tree.
+ my @tap_dirs = ();
+ my @top_dir = ($topdir);
+ File::Find::find(
+ { wanted => sub {
+ /^t\z/s
+ && $File::Find::name !~ /\/(kerberos|ldap|ssl|ssl_passphrase_callback)\// # opt-in
+ && push(@tap_dirs, $File::Find::name);
+ }
+ },
+ @top_dir);
+
+ $ENV{REGRESS_OUTPUTDIR} = "$topdir/src/test/recovery/tmp_check";
+ # Process each test
+ foreach my $test_path (@tap_dirs)
+ {
+ my $dir = dirname($test_path);
+ my $status = tap_check($dir);
+ $mstat ||= $status;
+ }
+ exit $mstat if $mstat;
+ return;
+}
+
sub taptest
{
my $dir = shift;
@@ -817,6 +851,7 @@ sub usage
print STDERR
"Usage: vcregress.pl <mode> [<arg>]\n\n",
"Options for <mode>:\n",
+ " alltaptests run all tap tests (except kerberos, ldap, ssl, ssl_passphrase_callback)\n",
" bincheck run tests of utilities in src/bin/\n",
" check deploy instance and run regression tests on it\n",
" contribcheck run tests of modules in contrib/\n",
@@ -834,6 +869,7 @@ sub usage
"\nOptions for <arg>: (used by contribcheck and modulescheck)\n",
" install also run tests which require a new instance\n",
"\nOption for <arg>: for taptest\n",
- " TEST_DIR (required) directory where tests reside\n";
+ " TEST_DIR (required) directory where tests reside\n",
+ " PROVE_FLAGS flags to pass to prove\n";
exit(1);
}
--
2.17.1
[text/x-diff] 0008-vcregress-run-alltaptests-in-parallel.patch (4.7K, ../20220213214213.GS31460@telsasoft.com/9-0008-vcregress-run-alltaptests-in-parallel.patch)
download | inline diff:
From fb066157b88823e256f94363c68a20b0568bfd45 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Tue, 18 Jan 2022 17:48:37 -0600
Subject: [PATCH 8/8] vcregress: run alltaptests in parallel
The test changes are needed to avoid these failures:
https://github.com/justinpryzby/postgres/runs/5174636590
[15:59:59.408] Bailout called. Further testing stopped: could not create test directory "c:/cirrus/tmp_check/t_C:\cirrus\src\bin\pgbench\t\002_pgbench_no_server_stuff": Invalid argument
[15:59:59.408] FAILED--Further testing stopped: could not create test directory "c:/cirrus/tmp_check/t_C:\cirrus\src\bin\pgbench\t\002_pgbench_no_server_stuff": Invalid argument
https://github.com/justinpryzby/postgres/runs/5174788205
[16:37:09.891] # Failed test 'reading traces/disallowed_in_pipeline.trace: could not open "traces/disallowed_in_pipeline.trace": The system cannot find the path specified at C:\cirrus\src\test\modules\libpq_pipeline\t\001_libpq_pipeline.pl line 70.
https://github.com/justinpryzby/postgres/runs/5174877506
pg_regress: could not open file "../regress/parallel_schedule" for reading: No such file or directory
XXX: avoid breaking src/bin/psql/t/010_tab_completion.pl
Should pass the srcdir to all taptests, or go back to writing the results to
tmp_check ?
See also:
f4ce6c4d3a30ec3a12c7f64b90a6fc82887ddd7b
795862c280c5949bafcd8c44543d887fd32b590a
db973ffb3ca43e65a0bf15175a35184a53bf977d
---
src/bin/pgbench/t/002_pgbench_no_server.pl | 4 ++--
.../libpq_pipeline/t/001_libpq_pipeline.pl | 5 ++++-
src/test/recovery/t/027_stream_regress.pl | 4 ++--
src/tools/msvc/vcregress.pl | 16 +++++-----------
4 files changed, 13 insertions(+), 16 deletions(-)
diff --git a/src/bin/pgbench/t/002_pgbench_no_server.pl b/src/bin/pgbench/t/002_pgbench_no_server.pl
index acad19edd0c..84e36dcd3fe 100644
--- a/src/bin/pgbench/t/002_pgbench_no_server.pl
+++ b/src/bin/pgbench/t/002_pgbench_no_server.pl
@@ -8,12 +8,12 @@
use strict;
use warnings;
+use File::Basename;
use PostgreSQL::Test::Utils;
use Test::More;
# create a directory for scripts
-my $testname = $0;
-$testname =~ s,.*/,,;
+my $testname = basename($0);
$testname =~ s/\.pl$//;
my $testdir = "$PostgreSQL::Test::Utils::tmp_check/t_${testname}_stuff";
diff --git a/src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl b/src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl
index 0c164dcaba5..facfec5cad4 100644
--- a/src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl
+++ b/src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl
@@ -49,7 +49,10 @@ for my $testname (@tests)
my $expected;
my $result;
- $expected = slurp_file_eval("traces/$testname.trace");
+ # Hack to allow TESTDIR=. during parallel tap tests
+ my $inputdir = "$ENV{'TESTDIR'}/src/test/modules/libpq_pipeline";
+ $inputdir = "$ENV{'TESTDIR'}" if ! -e $inputdir;
+ $expected = slurp_file_eval("$inputdir/traces/$testname.trace");
next unless $expected ne "";
$result = slurp_file_eval($traceout);
next unless $result ne "";
diff --git a/src/test/recovery/t/027_stream_regress.pl b/src/test/recovery/t/027_stream_regress.pl
index c0aae707ea1..4b84e3e78f5 100644
--- a/src/test/recovery/t/027_stream_regress.pl
+++ b/src/test/recovery/t/027_stream_regress.pl
@@ -58,9 +58,9 @@ system_or_bail($ENV{PG_REGRESS} . " $extra_opts " .
"--bindir= " .
"--host=" . $node_primary->host . " " .
"--port=" . $node_primary->port . " " .
- "--schedule=../regress/parallel_schedule " .
+ "--schedule=\"$dlpath/parallel_schedule\" " .
"--max-concurrent-tests=20 " .
- "--inputdir=../regress " .
+ "--inputdir=\"$dlpath\" " .
"--outputdir=\"$outputdir\"");
# Clobber all sequences with their next value, so that we don't have
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 4572db8eca4..b178713b911 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -299,8 +299,6 @@ sub alltaptests
{
InstallTemp();
- my $mstat = 0;
-
# Find out all the existing TAP tests by looking for t/ directories
# in the tree.
my @tap_dirs = ();
@@ -314,15 +312,11 @@ sub alltaptests
},
@top_dir);
- $ENV{REGRESS_OUTPUTDIR} = "$topdir/src/test/recovery/tmp_check";
- # Process each test
- foreach my $test_path (@tap_dirs)
- {
- my $dir = dirname($test_path);
- my $status = tap_check($dir);
- $mstat ||= $status;
- }
- exit $mstat if $mstat;
+ # Run all the tap tests in a single prove instance for good performance
+ $ENV{REGRESS_OUTPUTDIR} = "$topdir/src/test/recovery/tmp_check"; # For recovery check
+ $ENV{PROVE_TESTS} = "@tap_dirs";
+ my $status = tap_check('PROVE_FLAGS=--ext=.pl', "$topdir");
+ exit $status if $status;
return;
}
--
2.17.1
view thread (142+ messages) latest in thread
Message-ID: <20220213214213.GS31460@telsasoft.com>
Permalink: ../20220213214213.GS31460@telsasoft.com/
Also on: postgresql.org/message-id/20220213214213.GS31460@telsasoft.com
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: pryzby@telsasoft.com, andres@anarazel.de, tgl@sss.pgh.pa.us, robertmhaas@gmail.com, andrew@dunslane.net, thomas.munro@gmail.com, melanieplageman@gmail.com, peter.eisentraut@enterprisedb.com, daniel@yesql.se
Subject: Re: Adding CI to our tree
In-Reply-To: <20220213214213.GS31460@telsasoft.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox