On 24/07/2024 02:37, Michael Paquier wrote:
On Tue, Jul 23, 2024 at 08:32:29PM +0300, Heikki Linnakangas wrote:
All these new tests are a great asset when refactoring this again.
Thanks for doing that. The coverage, especially with v2, is going to
be really useful.
Yeah, I'm also not too excited about the additional code in the backend, but
I'm also not excited about writing another test C module just for this. I'm
inclined to commit this as it is, but we can certainly revisit this later,
since it's just test code.
The point would be to rely on the existing injection_points module,
with a new callback in it. The callbacks could be on a file of their
own in the module, for clarity.
Hmm, do we want injection_points module to be a dumping ground for
callbacks that are only useful for very specific injection points, in
specific tests? I view it as a more general purpose module, containing
callbacks that are useful for many different tests. Don't get me wrong,
I'm not necessarily against it, and it would be expedient, that's just
not how I see the purpose of injection_points.
What you have is OK for me anyway, it
is good to add more options to developers in this area and this gets
used in core. That's also enough to manipulate the stack in or even
out of core.
Ok, I kept it that way.
Here's a new rebased version with some minor cleanup. Notably, I added docs
for the new IS_INJECTION_POINT_ATTACHED() macro.
0001 looks OK.
+ push @events, "backenderror" if $line =~ /error triggered for
injection point backend-/;
+ push @events, "v2error" if $line =~ /protocol version 2 error
triggered/;
Perhaps append an "injection_" for these two keywords?
+#include "storage/proc.h"
This inclusion in injection_point.c should not be needed.
sets the FrontendProtocol global variable, but I think it's more
straightforward to have the test code
The last sentence in the commit message of 0002 seems to be
unfinished.
Fixed.
Could you run a perltidy on 005_negotiate_encryption.pl? There are a
bunch of new diffs in it.
Fixed.
Committed, thanks for the review, and thanks Jacob for the testing!
--
Heikki Linnakangas
Neon (https://neon.tech)
