On Mon, Mar 31, 2025 at 04:26:27PM -0500, Andrew Jackson wrote:
> I am working on a feature adjacent to the connection service functionality
> and noticed some issues with the tests introduced in this thread. Basically
> they incorrectly invoke the append perl function by passing multiple
> strings to append when the function only takes one string to append. This
> caused the generated service files to not actually contain any connection
> parameters. The tests were only passing because the connect_ok perl
> function set the connection parameters as environment variables which
> covered up the misformed connection service file.
Yep, you are right on this one. I didn't really like the hardcoding
of the host and port parts, and we should still be OK to rely on a
connstr from the valid node split on spaces. At least that's a bit
simpler.
> The attached patch is much more strict in that it creates a dummy database
> that is not started and passes all queries though that and tests that the
> connection service file correctly overrides the environment variables set
> by the dummy databases' query functions
Interesting trick, I like that. The point of not starting the node is
important, while we also make sure to load an environment related to
the node where the valid connection should happen. Breaking the
contents of the valid service file on purpose breaks the connection
attempts, making the tests fail.
So applied as you have proposed, mostly, and I have added more
documentation explaining the idea behind the dummy node.
--
Michael
