Hi, Rucha! Thanks! Looks good now. One comment about tests:
On Nov 18, Rucha Deodhar wrote: > revision-id: 4492c869f31 (mariadb-10.6.1-212-g4492c869f31) > parent(s): 3f3ec40c91b > author: Rucha Deodhar > committer: Rucha Deodhar > timestamp: 2021-11-08 16:51:33 +0530 > message: > > MDEV-26238: Remove inconsistent behaviour of --default-* options > in my_print_defaults ... > diff --git a/mysql-test/main/my_print_defaults.test > b/mysql-test/main/my_print_defaults.test > index bfd4e563826..b52deaf09ff 100644 > --- a/mysql-test/main/my_print_defaults.test > +++ b/mysql-test/main/my_print_defaults.test > @@ -87,20 +85,74 @@ read_buffer_size=1M ... > +--echo # checking that --defaults* option only works when mentioned at > beginning > + > +--echo # Testing --defaults-file at beginning only > +--exec $MYSQL_MY_PRINT_DEFAULTS > --defaults-file=$MYSQLTEST_VARDIR/tmp/tmp1.cnf --mysqld > +--remove_file $MYSQLTEST_VARDIR/tmp/tmp3.cnf How does it test that "--defaults* option only works at beginning"? You test that --defaults-file works at the beginning all right. This is redundant, as there are lots of tests in this file that have --defaults-file, but it doesn't hurt. But you don't test the "only" part. You don't test anything that you've changed - I suspect your new test would happily pass without any changes in my_print_defaults.c To test that --defaults-file works *only* at the beginning, you need to verify that it fails not at the beginning. Like error 1 exec $MYSQL_MY_PRINT_DEFAULTS --myslqd --defaults-file=$MYSQLTEST_VARDIR/tmp/tmp1.cnf also with -c error 1 exec $MYSQL_MY_PRINT_DEFAULTS -c $MYSQLTEST_VARDIR/tmp/tmp1.cnf --mysqld This should fail even at the beginning. The point is - you need to test the behavior that you've changed. As your test has introduced failures - you need to test for these failures. But all your tests succeed. > +--echo # Testing --defaults-extra-file works at beginning only > +--exec $MYSQL_MY_PRINT_DEFAULTS > --defaults-file=$MYSQLTEST_VARDIR/tmp/tmp1.cnf > --defaults-extra-file=$MYSQLTEST_VARDIR/tmp/tmp2.cnf --mysqld Regards, Sergei VP of MariaDB Server Engineering and [email protected] _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
