Re: PG_TEST_EXTRA and meson

Hi,

On Thu, 12 Sept 2024 at 12:35, Ashutosh Bapat
<ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
>
> Here's what I understand, please correct me: The code in meson.build
> is only called at the time of setup; not during meson test. Hence we
> can not check the existence of a runtime environment variable in that
> file. The things in test_env override those set at run time. So we
> save it as an argument to --pg_test_extra and then use it if
> PG_TEST_EXTRA is not set at run time. I tried to find if there's some
> other place to store "environment variables that can be overriden at
> runtime" but I can not find it. So it looks like this is the best we
> can do for now.

Yes, that is exactly what is happening.

> If it comes to a point where there are more such environment variables
> that need to be passed, probably we will pass a key-value string of
> those to testwrap. For now, for a single variable, this looks ok.

Yes, that would be better.

> > > +# If PG_TEST_EXTRA is not set in the environment, then look for its Meson
> > > +# configure option.
> > > +if "PG_TEST_EXTRA" not in os.environ and args.pg_test_extra:
> > > + env_dict["PG_TEST_EXTRA"] = args.pg_test_extra
> > > +
> > >
> > > If somebody looks at just these lines, it's not clear how
> > > PG_TEST_EXTRA is passed to the test environment if it's available in
> > > os.environ. So one has to understand that env_dict is the test
> > > environment. If that's so, the code and comment rewritten as below
> > > makes more sense to me. What do you think?
> > >
> > > # If PG_TEST_EXTRA is not already part of the test environment, check if it's
> > > # passed via program argument --pg_test_extra. Thus we also override
> > > # configuration time value by run time value of PG_TEST_EXTRA.
> > > if "PG_TEST_EXTRA" not in env_dict and args.pg_test_extra:
> > > env_dict["PG_TEST_EXTRA"] = args.pg_test_extra
> >
> > I think your suggestion is better, done.
>
> I didn't see the expected change. I was talking about something like attached.
>
> Also
> 1. I have also made changes to the comment,
> 2. renamed the argument --pg_test_extra to --pg-test-extra using
> convention similar to other arguments.
> 3. few other cosmetic changes.
>
> Please review and incorporate those in the respective patches and
> tests. Sorry for a single diff.
>
> Once this is done, I think we can mark this CF entry as RFC.

Thanks for the changes. I applied all of them in respective patches.

--
Regards,
Nazir Bilal Yavuz
Microsoft