Hi, On Tue, Jan 02, 2024 at 10:49:03PM -0500, Robert Haas wrote: > On Tue, Jan 2, 2024 at 4:45 PM Nathan Bossart <nathandboss...@gmail.com> > wrote: > > That seems to date back to commit 14a9101. I can agree that the suffix is > > somewhat redundant since these are already marked as type "LWLock", but > > I'll admit I've been surprised by this before, too. IMHO it makes this > > proposed test more important because you can't just grep for a different > > lock to find all the places you need to update. > > I agree. I am pretty sure that the reason this happened in the first > place is that I grepped for the name of some other LWLock and adjusted > things for the new lock at every place where that found a hit. > > > > - Check in both directions instead of just one? > > > > > > - Verify ordering? > > > > To do those things, I'd probably move the test to one of the scripts that > > generates the documentation or header file (pg_wait_events doesn't tell us > > whether a lock is predefined or what order it's listed in). That'd cause > > failures at build time instead of during testing, which might be kind of > > nice, too. > > Yeah, I think that would be better.
+1 to add a test and put in a place that would produce failures at build time. I think that having the test in the script that generates the header file is more appropriate (as building the documentation looks less usual to me when working on a patch). Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
