Redmine

It is a very interesting patch. But MailHandlerTest fails on my environment (Ruby 2.3).

$ ruby test/unit/mail_handler_test.rb
Run options: --seed 1974

# Running:

......................E

Error:
MailHandlerTest#test_truncate_emails_with_a_single_quoted_reply_should_truncate_the_email_at_the_delimiter_with_the_quoted_reply_symbols_(>):
NoMethodError: undefined method `match?' for #<String:0x007f91d84ebe90>
Did you mean?  match
    test/unit/mail_handler_test.rb:1005:in `block (2 levels) in <class:MailHandlerTest>'
    test/test_helper.rb:93:in `with_settings'
    test/unit/mail_handler_test.rb:1001:in `block in <class:MailHandlerTest>'

bin/rails test test/unit/mail_handler_test.rb:1000

........E

Error:
MailHandlerTest#test_truncate_emails_with_multiple_quoted_replies_should_truncate_the_email_at_the_delimiter_with_the_quoted_reply_symbols_(>):
NoMethodError: undefined method `match?' for #<String:0x007f91e3ad4d38>
Did you mean?  match
    test/unit/mail_handler_test.rb:1015:in `block (2 levels) in <class:MailHandlerTest>'
    test/test_helper.rb:93:in `with_settings'
    test/unit/mail_handler_test.rb:1011:in `block in <class:MailHandlerTest>'

bin/rails test test/unit/mail_handler_test.rb:1010

...........................................................

Finished in 21.213978s, 4.2896 runs/s, 20.2697 assertions/s.
91 runs, 430 assertions, 0 failures, 2 errors, 0 skips

The following sentence causes one of the error. Regexp.escape returns a String object. ActiveSupport defines Regexp#match? but does not String#match?.

      assert !Regexp.escape("--- Reply above. Do not remove this line. ---").match?(journal.notes)

You're right. I will revert lines with Regexp.escape tomorrow, but otherwise it should work.

+1
Redmine uses a lot of match methods.
I think it's wonderful that the test will be 5% faster.

Pavel Rosický wrote:

I combined all patches into a single file and I retested it on Ruby 2.3 and Ruby 2.4

Thank you for updating the patch. I have confirmed that the patch passes all test in r18003.

Regexp#match? is faster than Regexp#match or =~ in most cases because it does not allocate MatchData object. I think there is no reason not to merge this patch.