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 <hlinn...@iki.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) > > >
