Thanks Nazir,
> > -# Test suites that are not safe by default but can be run if selected
> > -# by the user via the whitespace-separated list in variable PG_TEST_EXTRA.
> > -# Export PG_TEST_EXTRA so it can be checked in individual tap tests.
> > -test_env.set('PG_TEST_EXTRA', get_option('PG_TEST_EXTRA'))
> >
> > A naive question. What happens if we add PG_TEST_EXTRA in meson.build
> > itself rather than passing it via testwrap? E.g. like
> > if "PG_TEST_EXTRA" not in os.environ
> > test_env.set('PG_TEST_EXTRA', get_option('PG_TEST_EXTRA'))
>
> Then this configure time option will be passed to the test environment
> and there is no way to change it without reconfiguring if we don't
> touch the testwrap file.
>
> > I am worried that we might have to add an extra argument to testwrap
> > for every environment variable that influences the tests. Avoiding it
> > would be better.
>
> If we want to have both configure time and run time variables, I
> believe that this is the only way for now.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. 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. > > > +# 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. -- Best Wishes, Ashutosh Bapat
pg_test_extra.diffs
Description: Binary data
