Defect #33562

Some tests in ApplicationHelperTest are declared as private

Added by Go MAEDA 5 months ago. Updated 5 months ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Go MAEDA% Done:

0%

Category:Code cleanup/refactoring
Target version:4.2.0
Resolution:Fixed Affected version:

Description

The attached patch fixes that the following two test methods are never executed.

  • test_export_csv_encoding_select_tag_should_return_nil_when_general_csv_encoding_is_UTF8
  • test_export_csv_encoding_select_tag_should_have_two_option_when_general_csv_encoding_is_not_UTF8

This is because a private keyword (source:tags/4.1.1/test/helpers/application_helper_test.rb#L1891) is placed before those two methods in r17490.

fix-unreachable-test-methods.patch Magnifier (1.91 KB) Go MAEDA, 2020-06-06 08:55


Related issues

Related to Redmine - Defect #31929: MarkdownFormatterTest#test_should_support_underlined_text... Closed

Associated revisions

Revision 19814
Added by Go MAEDA 5 months ago

Some tests in ApplicationHelperTest are declared as private (#33562).

Patch by Go MAEDA.

History

#1 Updated by Go MAEDA 5 months ago

  • Target version set to Candidate for next major release

#2 Updated by Mischa The Evil 5 months ago

  • Target version changed from Candidate for next major release to 4.2.0

LGTM. I don't see why this has to be postponed until the next major release (5.x.x). I think it can safely go into the next minor (4.2.0) release (hence, it could even go into 4.1.2/4.0.8, but I don't think that the significance of this fix warrants such a back-port).

FWIW: this is not the first case where this happened (see #31929), which brings two questions to my mind:
  1. Are there currently more, undetected cases like this?
  2. Is this something that, in the future, can be handled by (a custom) RuboCop (cop) now that's being used?

#3 Updated by Mischa The Evil 5 months ago

  • Related to Defect #31929: MarkdownFormatterTest#test_should_support_underlined_text is declared as private added

#4 Updated by Go MAEDA 5 months ago

Mischa The Evil wrote:

LGTM. I don't see why this has to be postponed until the next major release (5.x.x). I think it can safely go into the next minor (4.2.0) release (hence, it could even go into 4.1.2/4.0.8, but I don't think that the significance of this fix warrants such a back-port).

Thank you for reviewing the patch. My understanding is that the next major release is 4.2.0, and the next minor release is 4.1.2.

  1. Are there currently more, undetected cases like this?

I have just checked the following files and no other cases are found.

#5 Updated by Go MAEDA 5 months ago

  • Subject changed from Test methods that will never be executed in ApplicationHelperTest to Some tests in ApplicationHelperTest are declared as private
  • Assignee set to Go MAEDA
  • Resolution set to Fixed

Committed the patch.

#6 Updated by Go MAEDA 5 months ago

  • Status changed from New to Closed

#7 Updated by Mischa The Evil 5 months ago

Go MAEDA wrote:

[...] My understanding is that the next major release is 4.2.0, and the next minor release is 4.1.2.

That's why I mentioned it explicitly as there seems to be some confusion about this in the community.
FWIW: AFAIK Redmine follows the (semver) major.minor.patch scheme. So major releases are 3.x.x, 4.x.x, 5.x.x; minor releases are 4.1.x, 4.2.x, 4.3.x and 4.1.1, 4.1.2, 4.1.3 are patch releases.

This is documented in the (currently somewhat outdated) ReleaseManagement wiki page.

Also available in: Atom PDF