Re: Move tests of contrib/spi/ out of the core regression tests

Hi
> This is too late for v18 of course, so I'll park it in the next CF.

> In my opinion this would still be totally OK for v18. It's just test
> changes. I would rather commit cleanups like this sooner than later.
Agree +1

On Tue, Apr 8, 2025 at 5:22 PM Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:

> On 08/04/2025 04:59, Tom Lane wrote:
> > The attached patch removes test cases concerned with contrib/spi/
> > from the core regression tests and instead puts them into new
> > test files in contrib/spi/ itself.
>
> +1
>
> > My original motivation for looking at this was the discussion in
> > [1] about whether to remove contrib/spi/refint.c entirely, since
> > it's rather buggy and not a great example of our modern coding
> > practices. So I wondered whether the core test cases that use it
> > were contributing any significant amount of code coverage on the
> > core code. (Spoiler: nope.) But I think this is generally good
> > cleanup anyway, because it locates the test code for contrib/spi
> > where a person would expect to find that, and removes some rather
> > grotty coding in src/test/regress's Makefile and meson.build.
> > As a side benefit, it removes some small number of cycles from
> > the core tests, which seems like a good thing.
> >
> > The tests for the refint module are just moved over verbatim,
> > except for using CREATE EXTENSION instead of manual declaration
> > of the C functions. Also, I kept the tests for COMMENT ON TRIGGER
> > in the core tests, by applying them to a different trigger.
> >
> > The tests for autoinc were kind of messy, because the behavior of
> > autoinc itself was impossibly intertwined with the behavior of
> > "ttdummy", which is an undocumented C function in regress.c.
> > After some thought I decided we could just nuke ttdummy altogether,
> > so the new autoinc.sql test is much simpler and more straightforward.
>
> My only worry would if 'ttdummy' was a good showcase for how to write a
> trigger function in C. But it's not a particularly good example. There
> is a better example in the docs, and there's the 'autoinc' trigger too.
>
> > This is too late for v18 of course, so I'll park it in the next CF.
>
> In my opinion this would still be totally OK for v18. It's just test
> changes. I would rather commit cleanups like this sooner than later.
>
> --
> Heikki Linnakangas
> Neon (https://neon.tech)
>
>
>