pg_regress: schedule multi-line test groups
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253819psql -h localhost -U postgresThis image is from patchset v3 (message #3) - the current patchset v5 (message #5) has not produced an image.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t253819_5 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t253819_5 && git checkout t253819_5Patchset v5 (message #5) is on t253819_5
Hi Hackers,
This was motivated by two rebases on the SLOPE patch [1] this week due to
changes in parallel_schedule. Also I find much easier to parse the list
using
this syntax, if we stick with one test per line we get the benefit of
counting
tests by looking at the line numbers.
I used a `goto` to keep the number of touched lines small, and facilitate
your review.
Attachments:
v1-0001-pg_regress-multi-line-test-groups.patchapplication/octet-stream; name=v1-0001-pg_regress-multi-line-test-groups.patchDownload+278-26
On Thu, Sep 17, 2026 at 08:57:51AM +0100, Alexandre Felipe wrote:
This was motivated by two rebases on the SLOPE patch [1] this week due to
changes in parallel_schedule. Also I find much easier to parse the list
using this syntax, if we stick with one test per line we get the benefit of
counting tests by looking at the line numbers.I used a `goto` to keep the number of touched lines small, and facilitate
your review.
I'd +1 something like that in the schedule files, even if we only
limit the number of tests per line to be up to 20.
+test:
+ boolean
+ char
+ name
At least, let's make the whitespace vs tab policy consistent. ;)
--
Michael
On Thu, Sep 17, 2026 at 9:03 AM Michael Paquier <michael@paquier.xyz> wrote:
On Thu, Sep 17, 2026 at 08:57:51AM +0100, Alexandre Felipe wrote:
This was motivated by two rebases on the SLOPE patch [1] this week due to
changes in parallel_schedule. Also I find much easier to parse the list
using this syntax, if we stick with one test per line we get the benefitof
counting tests by looking at the line numbers.
I used a `goto` to keep the number of touched lines small, and facilitate
your review.
I'd +1 something like that in the schedule files, even if we only
limit the number of tests per line to be up to 20.
Actually, I think we should support arbitrary size test groups and run them
on a pool
from a queue that would possibly make the regression faster as today we
always
wait for the slowest test in each group.
+test: + boolean + char + nameAt least, let's make the whitespace vs tab policy consistent. ;)
I will blame the editor, I hit tab and it adds spaces :) I started manually
but then I saw that
what I was doing was dumb and error prone, and did the rest with find and
replace (with \t)
now all using spaces.
Regards,
Alexandre
On 17 Sep 2026, at 10:43, Alexandre Felipe <o.alexandre.felipe@gmail.com> wrote:
Actually, I think we should support arbitrary size test groups and run them on a pool
from a queue that would possibly make the regression faster as today we always
wait for the slowest test in each group.
I know of ongoing, as of yet unpublished, work in this area to improve this
scheduling which will also change the syntax. There is a bit complexity to it
than just picking from a pool since there are inter-suite dependencies.
+ /*
+ * Found `test: # no tests` treat it as a multiline test group
+ */
if (num_tests == 0)
{
- bail("syntax error in schedule file \"%s\" line %d: %s",
- schedule, line_num, scbuf);
+ multiline_test = true;
+ }
This will allow empty test groups which we currently treat as an error. This
schedule:
test: test_setup\n
test:\n
\n
test:\n
Parses and runs with all tests succeeded:
# initializing database system by copying initdb template
# using temp instance on port 40058 with PID 71845
ok 1 - test_setup 448 ms
# parallel group (0 tests):
# parallel group (0 tests):
1..1
# All 1 tests passed.
# test succeeded
Without the patch an empty group will throw an error, and I think it makes
sense to preserve that behaviour.
Also, if we are to change the accepted syntax, why limit to single line or
multiline, why not accept any whitespace separated name between ^test: and
(^test|EOF)? Something like the below:
test: boolean
char
name
--
Daniel Gustafsson
Thank you for your looking into it Daniel
On Fri, Sep 18, 2026 at 10:31 AM Daniel Gustafsson <daniel@yesql.se> wrote:
On 17 Sep 2026, at 10:43, Alexandre Felipe <o.alexandre.felipe@gmail.com>
wrote:
Actually, I think we should support arbitrary size test groups and run
them on a pool
from a queue that would possibly make the regression faster as today we
always
wait for the slowest test in each group.
I know of ongoing, as of yet unpublished, work in this area to improve this
scheduling which will also change the syntax. There is a bit complexity
to it
than just picking from a pool since there are inter-suite dependencies.
An easy gain would be to somehow transpose execution, currently we put
together
things to indicate that they are independent, it would be more readable if
we used
the lines to indicate dependency.
The right way to handle dependency is by writing a dependency tree and doing
a topological sort, like makefiles, but to keep the syntax backward
compatible
we could do something like this
# tests that have to run in isolation
test: setup_test
test: sanity_check
# tests that have to respect certain order
test sequentially: create_am psql
# where a test should wait for multiple tests that can run in parallel
test sequentially: (geometry create_index_spgist hash_index brin) amutils
# tests that can mix with other tests
test concurrently: compression compression_lz4 compression_pglz cluster
But please, let's first get the multiline syntax over the fence.
v2 is logging the effective concurrency of the groups during the execution
(we can't optimise what we don't measure).
e.g:
# effective concurrency 5.80 / 18
# effective concurrency 3.83 / 15
+ /*
+ * Found `test: # no tests` treat it as a multiline test group + */ if (num_tests == 0) { - bail("syntax error in schedule file \"%s\" line %d: %s", - schedule, line_num, scbuf); + multiline_test = true; + }This will allow empty test groups which we currently treat as an error.
This
schedule:
Fixed, also improved the error messages
Also, if we are to change the accepted syntax, why limit to single line or
multiline, why not accept any whitespace separated name between ^test: and
(^test|EOF)? Something like the below:test: boolean
char
name
Minimalism. in v2 I am trying to do something more like what you described
The schedule syntax in v2 should be
* Schedule = (Blank | Comment | Group)*
* Group = "test:" (Line | Comment) (Indent Line)*
* Line = token (Space+ token)* Comment?
* Blank = '\n'
* Comment = '#' [^\n]* \n
* Indent = [\t ]+
But I won't be surprised if you find inconsistencies between that and what
was actually implemented at this stage.
Regards,