jorgenschaefer / emacs-buttercup

Behavior-Driven Emacs Lisp Testing
GNU General Public License v3.0
362 stars 44 forks source link

Change: (buttercup-define-matcher-for-binary-function) Add newlines #159

Closed alphapapa closed 4 years ago

alphapapa commented 4 years ago

Add newlines to expect-match-phrase to make output human-readable.

Here's some example output before the change:

FAILED: Expected `(org-ql org-ql-test-buffer (not (tags-local
"personal" "world")) :action (org-ql-test-org-get-heading))' to be `equal' to `("Test data" "Take over the universe" "Take over Mars" "Visit Mars"
"Take over the moon" "Visit the moon" "Renew membership in supervillain club" "Learn universal sign language" "Order a pizza" "Internet" "Spaceship
lease" "Fix flux capacitor" "Recurring" "/r/emacs" "Shop for groceries" "Sunrise/sunset" "Ideas" "Rewrite Emacs in Common Lisp" "Write a symphony"
"Code" "Agenda examining" "Agenda censoring" "Auto grouping" "Auto categories" "Date" "Effort" "Misc" "let-plist" "Profiling")', but instead it was
`("Test data" "Take over the universe" "Take over Mars" "Visit Mars" "Take over the moon" "Visit the moon" "Renew membership in supervillain club"
"Learn universal sign language" "Order a pizza" "Internet" "Spaceship lease" "Fix flux capacitor" "Recurring" "/r/emacs" "Shop for groceries"
"Sunrise/sunset" "Ideas" "Rewrite Emacs in Common Lisp" "Write a symphony")' which does not match because: (proper-lists-of-different-length 19 29
("Test data" "Take over the universe" "Take over Mars" "Visit Mars" "Take over the moon" "Visit the moon" "Renew membership in supervillain club"
"Learn universal sign language" "Order a pizza" "Internet" "Spaceship lease" "Fix flux capacitor" "Recurring" "/r/emacs" "Shop for groceries"
"Sunrise/sunset" "Ideas" "Rewrite Emacs in Common Lisp" "Write a symphony") ("Test data" "Take over the universe" "Take over Mars" "Visit Mars" "Take
over the moon" "Visit the moon" "Renew membership in supervillain club" "Learn universal sign language" "Order a pizza" "Internet" "Spaceship lease"
"Fix flux capacitor" "Recurring" "/r/emacs" "Shop for groceries" "Sunrise/sunset" "Ideas" "Rewrite Emacs in Common Lisp" "Write a symphony" "Code"
"Agenda examining" "Agenda censoring" "Auto grouping" "Auto categories" "Date" "Effort" "Misc" "let-plist" "Profiling") first-mismatch-at 19).

And after the change:

FAILED: Expected:
    `(org-ql org-ql-test-buffer (not (tags-local "personal" "world")) :action (org-ql-test-org-get-heading))'

to be `equal' to:
    `("Test data" "Take over the universe" "Take over Mars" "Visit Mars" "Take over the moon" "Visit the moon" "Renew membership in supervillain club"
"Learn universal sign language" "Order a pizza" "Internet" "Spaceship lease" "Fix flux capacitor" "Recurring" "/r/emacs" "Shop for groceries"
"Sunrise/sunset" "Ideas" "Rewrite Emacs in Common Lisp" "Write a symphony" "Code" "Agenda examining" "Agenda censoring" "Auto grouping" "Auto
categories" "Date" "Effort" "Misc" "let-plist" "Profiling")'

but instead it was:
    `("Test data" "Take over the universe" "Take over Mars" "Visit Mars" "Take over the moon" "Visit the moon" "Renew membership in supervillain club"
"Learn universal sign language" "Order a pizza" "Internet" "Spaceship lease" "Fix flux capacitor" "Recurring" "/r/emacs" "Shop for groceries"
"Sunrise/sunset" "Ideas" "Rewrite Emacs in Common Lisp" "Write a symphony")'

which does not match because:
    (proper-lists-of-different-length 19 29 ("Test data" "Take over the universe" "Take over Mars" "Visit Mars" "Take over the moon" "Visit the moon"
"Renew membership in supervillain club" "Learn universal sign language" "Order a pizza" "Internet" "Spaceship lease" "Fix flux capacitor" "Recurring"
"/r/emacs" "Shop for groceries" "Sunrise/sunset" "Ideas" "Rewrite Emacs in Common Lisp" "Write a symphony") ("Test data" "Take over the universe"
"Take over Mars" "Visit Mars" "Take over the moon" "Visit the moon" "Renew membership in supervillain club" "Learn universal sign language" "Order a
pizza" "Internet" "Spaceship lease" "Fix flux capacitor" "Recurring" "/r/emacs" "Shop for groceries" "Sunrise/sunset" "Ideas" "Rewrite Emacs in Common
Lisp" "Write a symphony" "Code" "Agenda examining" "Agenda censoring" "Auto grouping" "Auto categories" "Date" "Effort" "Misc" "let-plist"
"Profiling") first-mismatch-at 19).

As you can see, the second example is much more readable, and a human can easily see where the list of strings begins to differ.

This may not be the only place in the code where this kind of change would be helpful, but it solves my current need, so I'm only proposing this one.

Thanks.

snogge commented 4 years ago

I reverted this change, as the side effects were not very pleasant. See the commit message of b297b1dbfa21c87ffbcfc12d19262765387848de.

A better way of formatting the messages would undoubtably be nice, but this was unfortunately not it.

alphapapa commented 4 years ago

Thanks. So if I'm understanding correctly, the problem is that the call to buttercup-fail is itself included in the traceback, which duplicates the failure message in the output. That was already a problem, but the newlines in the change exacerbate it. Right?

snogge commented 4 years ago

Yes, thats right. I guess we could get similar problems in the backtrace without this change, but I don't remember any so extreme. Maybe we can solve it with the backtrace formatting functions.