From: Josh Steadmon Date: Mon, 01 Jul 2024 20:32:30 GMT Subject: Re: [GSoC][PATCH v2] t: migrate helper/test-oidmap.c to unit-tests/t-oidmap.c Message-ID: In-Reply-To: <20240628122030.41554-1-shyamthakkar001@gmail.com> On 2024.06.28 17:50, Ghanshyam Thakkar wrote: > helper/test-oidmap.c along with t0016-oidmap.sh test the oidmap.h > library which is built on top of hashmap.h. > > Migrate them to the unit testing framework for better performance, > concise code and better debugging. Along with the migration also plug > memory leaks and make the test logic independent for all the tests. > The migration removes 'put' tests from t0016, because it is used as > setup to all the other tests, so testing it separately does not yield > any benefit. > > Helped-by: Phillip Wood > Mentored-by: Christian Couder > Mentored-by: Kaartic Sivaraam > Signed-off-by: Ghanshyam Thakkar > --- > This version addresses Phillip's review about detecting duplicates in > oidmap when iterating over it and removing put_and_check_null() to move > the relevant code to setup() instead. And contains some grammer fixes > in the comment. IIUC this corrects all of the issues that Phillip noted in his earlier review, except for checking for duplicates, is that right? Personally I think this version is OK even without that check, and I'll be away from email for the rest of this week, so I'll go ahead and sign off: Reviewed-by: Josh Steadmon