maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

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 security@xxxxxxxxxxx