public inbox for [email protected]
help / color / mirror / Atom feedFrom: Andres Freund <[email protected]>
To: Justin Pryzby <[email protected]>
Cc: Andrew Dunstan <[email protected]>
Cc: Thomas Munro <[email protected]>
Cc: [email protected]
Cc: Noah Misch <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Anastasia Lubennikova <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Melanie Plageman <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Cc: samay sharma <[email protected]>
Subject: Re: CI and test improvements
Date: Tue, 17 Jan 2023 11:56:42 -0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
Hi,
On 2023-01-17 11:35:09 -0600, Justin Pryzby wrote:
> The autoconf system runs all tap tests in t/*.pl, but meson requires
> enumerating them in ./meson.build.
Yes. It was a mistake that we ever used t/*.pl for make. For one, it means
that make can't control concurrency meaningfully, due to the varying number of
tests run with one prove instance. It's also the only thing that tied us to
prove, which is one hell of a buggy mess.
> This checks for and finds no missing tests in the current tree:
>
> $ for pl in `find src contrib -path '*/t/*.pl'`; do base=${pl##*/}; dir=${pl%/*}; meson=${dir%/*}/meson.build; grep "$base" "$meson" >/dev/null || echo "$base is missing from $meson"; done
Likely because I do something similar locally.
# prep
m test --list > /tmp/tests.txt
# Check if all tap tests are known to meson
for f in $(git ls-files|grep -E '(t|test)/.*.pl$'|sort);do t=$(echo $f|sed -E -e 's/^.*\/([^/]*)\/(t|test)\/(.*)\.pl$/\1\/\3/');grep -q -L $t /tmp/tests.txt |\
| echo $f;done
# Check if all regression / isolation tests are known to meson
#
# Expected to find plpgsql due to extra 'src' directory level, src/test/mb
# because it's not run anywhere and sepgsql, because that's not tested yet
for d in $(find ~/src/postgresql -type d \( -name sql -or -name specs \) );do t=$(basename $(dirname $d)); grep -q -L $t /tmp/tests.txt || echo $d; done
> However, this finds two real problems and one false-positive with
> missing regress/isolation tests:
Which the above does *not* test for. Good catch.
I'll push the fix for those as soon as tests passed on my personal repo.
> $ for makefile in `find src contrib -name Makefile`; do for testname in `sed -r '/^(REGRESS|ISOLATION) =/!d; s///; :l; /\\\\$/{s///; N; b l}; s/\n//g' "$makefile"`; do meson=${makefile%/Makefile}/meson.build; grep -Fw "$testname" "$meson" >/dev/null || echo "$testname is missing from $meson"; done; done
> guc_privs is missing from src/test/modules/unsafe_tests/meson.build
Yep. That got added during the development of the meson port, so it's not too surprising.
> oldextversions is missing from contrib/pg_stat_statements/meson.build
This one however, is odd. Not sure how that happened.
> $(CF_PGP_TESTS) is missing from contrib/pgcrypto/meson.build
Assume that's the false positive?
> I also tried but failed to write something to warn if "meson test" was
> run with a list of tests but without tmp_install. Help wanted.
That doesn't even catch the worst case - when there's tmp_install, but it's
too old.
The proper solution would be to make the creation of tmp_install a dependency
of the relevant tests. Unfortunately meson still auto-propages those to
dependencies of the 'all' target (for historical reasons), and creating the
temp install is too slow on some machines to make that tolerable. I think
there's an open PR to change that. Once that's in a released meson version
that's in somewhat widespread use, we should change that.
The other path forward is to allow running the tests without
tmp_install. There's not that much we'd need to allow running directly from
the source tree - the biggest thing is a way to load extensions from a list of
paths. This option is especially attractive because it'd allow to run
individual tests without a fully built sourcetree. No need to build other
binaries when you just want to test psql, or more extremely, pg_test_timing.
> I propose to put something like this into "SanityCheck".
Perhaps we instead could add it as a separate "meson-only" test? Then it'd
fail on developer's machines, instead of later in CI. We could pass the test
information from the 'tests' array, or it could look at the metadata in
meson-info/intro-tests.json
Greetings,
Andres Freund
view thread (4+ messages)
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: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: CI and test improvements
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox