Defect #33562: Some tests in ApplicationHelperTest are declared as private - Redmine

Defect #33562

Some tests in ApplicationHelperTest are declared as private

Added by Go MAEDA over 1 year ago. Updated over 1 year ago.

Status:

Closed

Start date:

Priority:

Normal

Due date:

Assignee:

Go MAEDA

% Done:

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 (1.91 KB) Go MAEDA, 2020-06-06 08:55

Related issues

Associated revisions

Revision 19814
Added by Go MAEDA over 1 year ago

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

Patch by Go MAEDA.

History

#1 Updated by Go MAEDA over 1 year ago

Target version set to Candidate for next major release

#2 Updated by Mischa The Evil over 1 year 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:

Are there currently more, undetected cases like this?
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 over 1 year ago

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

#4 Updated by Go MAEDA over 1 year 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.

Are there currently more, undetected cases like this?

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

Files Files

$ grep -lr "^\s*private\s*$" test
test/mocks/open_id_authentication_mock.rb
test/unit/project_nested_set_test.rb
test/unit/mailer_localisation_test.rb
test/unit/mailer_test.rb
test/unit/version_test.rb
test/unit/search_test.rb
test/unit/mail_handler_test.rb
test/unit/repository_bazaar_test.rb
test/unit/activity_test.rb
test/unit/lib/redmine/configuration_test.rb
test/unit/lib/redmine/unified_diff_test.rb
test/unit/lib/redmine/scm/adapters/subversion_adapter_test.rb
test/unit/lib/redmine/scm/adapters/bazaar_adapter_test.rb
test/unit/lib/redmine/scm/adapters/cvs_adapter_test.rb
test/unit/lib/redmine/scm/adapters/mercurial_adapter_test.rb
test/unit/lib/redmine/scm/adapters/git_adapter_test.rb
test/unit/lib/redmine/wiki_formatting/markdown_formatter_test.rb
test/unit/lib/redmine/wiki_formatting/textile_formatter_test.rb
test/unit/issue_nested_set_concurrency_test.rb
test/unit/repository_mercurial_test.rb
test/integration/attachments_test.rb
test/integration/sudo_mode_test.rb
test/system/sudo_mode_test.rb
test/helpers/application_helper_test.rb
test/helpers/sort_helper_test.rb
test/coverage/html_formatter.rb
test/functional/repositories_git_controller_test.rb
test/functional/admin_controller_test.rb
test/functional/timelog_custom_fields_visibility_test.rb

#5 Updated by Go MAEDA over 1 year 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 over 1 year ago

Status changed from New to Closed

#7 Updated by Mischa The Evil over 1 year 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

Redmine

Issues

Custom queries