Skip to content

Commit

Permalink
Fix cider-test--string-contains-newline
Browse files Browse the repository at this point in the history
  • Loading branch information
@vemv
vemv committed Aug 4, 2023
1 parent 1dc2c80 commit 605e253
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 1 deletion.
2 changes: 1 addition & 1 deletion cider-test.el
View file
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ With the actual value, the outermost '(not ...)' s-expression is removed."
(defun cider-test--string-contains-newline (input-string)
"Returns whether INPUT-STRING contains an escaped newline."
(when (stringp input-string)
(and (string-match-p "\\n" input-string)
(and (string-match-p "\\\\n" input-string)
t)))

(defun cider-test-render-assertion (buffer test)
Expand Down
3 changes: 3 additions & 0 deletions test/cider-test-tests.el
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@
(require 'cider-test)

(describe "cider-test--string-contains-newline"
(expect (cider-test--string-contains-newline "n")
:to-equal
nil)
(expect (cider-test--string-contains-newline "Hello\nWorld")
:to-equal
nil)
Expand Down

5 comments on commit 605e253

@bbatsov
Copy link
Member

@bbatsov bbatsov commented on 605e253 Aug 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing the other tests are not correct if they passed with the wrong regex.

@vemv
Copy link
Member Author

@vemv vemv commented on 605e253 Aug 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This "test suite" was correct, but incomplete 🌀

With the added test case, the existing tests are more useful than before.

@bbatsov
Copy link
Member

@bbatsov bbatsov commented on 605e253 Aug 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just wondering what newline was matched in the second (now third) test if the regexp was incorrect.

@vemv
Copy link
Member Author

@vemv vemv commented on 605e253 Aug 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because a escaped newline has the character n (as matched by the old buggy impl)

@bbatsov
Copy link
Member

@bbatsov bbatsov commented on 605e253 Aug 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I reliazed just now this is not an Elisp string newline.

Please sign in to comment.