git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH/RFC v4 07/10] send-email: reduce dependancies impact on parse_address_line

From
Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date
Jun 19, 2015, 07:16 UTC
Message-ID
<vpqpp4sw4ki.fsf@anie.imag.fr>
In-Reply-To
<114284546.628903.1434662953414.JavaMail.zimbra@ensimag.grenoble-inp.fr>
Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr> writes:
Show 10 quoted lines
> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> Cool. Then almost all the work is done to get an automated test. Next
>> step would be to add the tests itself in the code. I would do that by
>> adding a hidden --selfcheck option to git send-email that would compare
>> Mail::Address->parse($string); and split_addrs($string); for all your
>> testcases, and die if they do not match. Then calling it from the
>> testsuite would be trivial.
>
> Ok, are there such "--selfcheck" options elsewhere?
Not as far as I know.
> If I understand it right, you want to put the tests inside the
> git-send-email script. I don't feel really good about that but I guess
> it's hard to test it otherwise...

Hmm, actually there is, I didn't look at the right places yesterday. git-send-email.perl already does 'use Git;', and there's already a set of unit-tests for Git.pm: t9700-perl-git.sh, which calls perl "$TEST_DIRECTORY"/t9700/test.pl.

So, you can just add your code as a function in Git.pm and unit-tests in t/t9700/test.pl.

Show 5 quoted lines
> Also what will we do with the failing tests? Just discard them? I
> think there's two sort of failing test:
>
>  - When output provided by parse_address_ without Mail::Address
>    is better or has no impact at all on the code. Such as:

I'm not sure we can be "better" as long as we do use Mail::Address when available. Any difference is potentially harmfull for the user because it means that Git will have different behavior on different machines.

Perhaps this is an argument to use your version unconditionally and drop Mail::Address actually.

But you can still test that with
  is(parse_address_(...), "Doe, Jane", "<description>");
(possibly not calling Mail::Address)
http://search.cpan.org/~exodist/Test-Simple-1.001014/lib/Test/More.pm

The cases where Mail::Address and your version give the same result can be tested with a foreach loop calling

  is(parse_address_(...), Mail::Address(...), ...);
>  - When we don't really care about the output, because the user entry
>    is wrong, and we just expect the script to be aborted somehow... We
>    don't need to test that.

... but if you already have the tests, you can keep them as known failure.

See the "TODO: BLOCK" section of the doc of Test::More.
>> I can do that on top of your series if you don't have time.
>
> Time will become a problem soon, but I think I can handle it unless
> you really want to do it !
If you have time, just do it.
Thanks,
-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Previous: Remi LespinetNext: Remi Lespinet
Message 17 of 50 in “t9001-send-email: move script creation in a setup test”
  1. 01/10 t9001-send-email: move script creation in a setup testRemi Lespinet, Jun 17, 2015
  2. 02/10 send-email: allow aliases in patch header and command script outputsRemi Lespinet, Jun 17, 2015
  3. 03/10 t9001-send-email: refactor header variable fields replacementRemi Lespinet, Jun 17, 2015
  4. 04/10 send-email: refactor address list processRemi Lespinet, Jun 17, 2015
  5. 05/10 send-email: Allow use of aliases in the From field of --compose modeRemi Lespinet, Jun 17, 2015
  6. Matthieu MoyJun 17, 2015
  7. 06/10 send-email: minor code refactoringRemi Lespinet, Jun 17, 2015
  8. 07/10 send-email: reduce dependancies impact on parse_address_lineRemi Lespinet, Jun 17, 2015
  9. Matthieu MoyJun 17, 2015
  10. 07/10 send-email: reduce dependancies impact on parse_address_lineRemi Lespinet, Jun 17, 2015
  11. Junio C HamanoJun 17, 2015
  12. 07/10 send-email: reduce dependancies impact on parse_address_lineRemi Lespinet, Jun 17, 2015
  13. Matthieu MoyJun 18, 2015
  14. 07/10 send-email: reduce dependancies impact on parse_address_lineRemi Lespinet, Jun 18, 2015
  15. Matthieu MoyJun 18, 2015
  16. 07/10 send-email: reduce dependancies impact on parse_address_lineRemi Lespinet, Jun 18, 2015
  17. Matthieu MoyJun 19, 2015
  18. 08/10 send-email: consider quote as delimiter instead of characterRemi Lespinet, Jun 17, 2015
  19. 09/10 send-email: allow multiple emails using --cc, --to and --bccRemi Lespinet, Jun 17, 2015
  20. 10/10 send-email: suppress meaningless whitespaces in from fieldRemi Lespinet, Jun 17, 2015
  21. Matthieu MoyJun 17, 2015
  22. 10/10 send-email: suppress meaningless whitespaces in from fieldRemi Lespinet, Jun 17, 2015
  23. 01/10 t9001-send-email: move script creation in a setup testRemi Lespinet, Jun 20, 2015
  24. 02/10 send-email: allow aliases in patch header and command script outputsRemi Lespinet, Jun 20, 2015
  25. 03/10 t9001-send-email: refactor header variable fields replacementRemi Lespinet, Jun 20, 2015
  26. 04/10 send-email: refactor address list processRemi Lespinet, Jun 20, 2015
  27. 05/10 send-email: Allow use of aliases in the From field of --compose modeRemi Lespinet, Jun 20, 2015
  28. 06/10 send-email: minor code refactoringRemi Lespinet, Jun 20, 2015
  29. 07/10 send-email: reduce dependancies impact on parse_address_lineRemi Lespinet, Jun 20, 2015
  30. Matthieu MoyJun 21, 2015
  31. 07/10 send-email: reduce dependancies impact on parse_address_lineRemi Lespinet, Jun 21, 2015
  32. 07/10 send-email: reduce dependancies impact on parse_address_lineRemi Lespinet, Jun 23, 2015
  33. Matthieu MoyJun 21, 2015
  34. 08/10 send-email: consider quote as delimiter instead of characterRemi Lespinet, Jun 21, 2015
  35. 09/10 send-email: allow multiple emails using --cc, --to and --bccRemi Lespinet, Jun 21, 2015
  36. Matthieu MoyJun 21, 2015
  37. 10/10 send-email: suppress meaningless whitespaces in from fieldRemi Lespinet, Jun 21, 2015
  38. 01/10 t9001-send-email: move script creation in a setup testRemi Lespinet, Jun 23, 2015
  39. 02/10 send-email: allow aliases in patch header and command script outputsRemi Lespinet, Jun 23, 2015
  40. 03/10 t9001-send-email: refactor header variable fields replacementRemi Lespinet, Jun 23, 2015
  41. 04/10 send-email: refactor address list processRemi Lespinet, Jun 23, 2015
  42. 05/10 send-email: Allow use of aliases in the From field of --compose modeRemi Lespinet, Jun 23, 2015
  43. 06/10 send-email: minor code refactoringRemi Lespinet, Jun 23, 2015
  44. 07/10 send-email: reduce dependencies impact on parse_address_lineRemi Lespinet, Jun 23, 2015
  45. Matthieu MoyJun 23, 2015
  46. 07/10 send-email: reduce dependencies impact on parse_address_lineRemi LESPINET, Jun 23, 2015
  47. 08/10 send-email: consider quote as delimiter instead of characterRemi Lespinet, Jun 23, 2015
  48. 09/10 send-email: allow multiple emails using --cc, --to and --bccRemi Lespinet, Jun 23, 2015
  49. Matthieu MoyJun 23, 2015
  50. 10/10 send-email: suppress meaningless whitespaces in from fieldRemi Lespinet, Jun 23, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.