Re: Move tests of contrib/spi/ out of the core regression tests
От | wenhui qiu |
---|---|
Тема | Re: Move tests of contrib/spi/ out of the core regression tests |
Дата | |
Msg-id | CAGjGUAKjMcaQ0eQvqnbPXQ+v4V-v9JEqFAiR5vFy1_GsTgG8MA@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: Move tests of contrib/spi/ out of the core regression tests (Heikki Linnakangas <hlinnaka@iki.fi>) |
Список | pgsql-hackers |
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
> 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@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)
В списке pgsql-hackers по дате отправления: