Richard Hansen via Postfix-devel:
> I left a TODO comment in the second patch questioning whether $db:$name tables
> inside unionmap: and pipemap: table names should also be handled there.
> Please
> let me know if my understanding of that code is off.
We certainly want to make sure that debug: prefixes are handled as
expected in pseudo dictionaries (union, pipe). But that is just to
confirm that debug: prefixes are handled; there is no need for
extensive coverage of every debug: behavior.
> > - Need to add ${SHLIB_ENV} before ${VALGRIND} in tests, so that
> > they will work when Postfix is built with shared libpostfix-*
> > libraries.
>
> That was already in place in the Makefile. I can move it to the shell script,
> but that gets a bit awkward with the .ref file and `set +x`. (The same is
> true
> for ${VALGRIND}, but I figured that Valgrind is manually run so the .ref
> mismatch isn't a big deal. Let me know if I should refactor to move
> ${VALGRIND}
> out of the shell script.)
I have some general comments:
First, The Valgrind tests are NOT NMANUAL. There are lots of tests
and running them by hand is not an option. Please move the
dict_debug_test.sh commands (and any others that you added) into
the Makefile.in files just like the other tests, or use "$(SHLIB_ENV)
${VALGRIND} sh -x" in the Makefile.in files.
[I see that I violated my own rule a few days ago when I added
the mode_conflict_test targets to the postmap and postalias
Makefile.in files. Please don't attempt to fix those. I will
take care of them myself.]
Second, your changes to the existing tests defeat the purpose of
those tests. The primary purpose of EXISTING tests is to ensure
that after a feature is added, no EXISTING behavior is changed.
Changes to existng test files will cause the code review to take a
much longer time, other work will take precedence, and the patch
may never be adopted.
Please add new tests to verify new behavior, and don't modify
existing tests.
I'll send more detailed comments after reviewing V3.
Wietse
_______________________________________________
Postfix-devel mailing list -- postfix-devel@postfix.org
To unsubscribe send an email to postfix-devel-le...@postfix.org
