Defect #36361: IssueRelationsControllerTest#test_bulk_create_should_show_errors randomly fails - Redmine

Actions

Copy link

Defect #36361

closed

IssueRelationsControllerTest#test_bulk_create_should_show_errors randomly fails

Added by Yuichi HARADA over 2 years ago. Updated over 2 years ago.

Status:

Closed

Priority:

Normal

Assignee:

Go MAEDA

Category:

Code cleanup/refactoring

Target version:

5.0.0

Start date:

Due date:

% Done:

Estimated time:

Resolution:

Fixed

Affected version:

Description

The test will be always failed if the following conditions are met:

using trunk(r21326)
Run the test specified a seed value( TESTOPTS="--seed 21802" )

% RAILS_ENV=test bundle exec rake db:migrate:reset

% RAILS_ENV=test bundle exec rake test TESTOPTS="--seed 21802" 
(Test LDAP server not configured)
Bazaar non ASCII output test cannot run on this environment.
Encoding.locale_charmap: UTF-8
Skipping LDAP tests.
Run options: --seed 21802

# Running:

.................F

Failure:
IssueRelationsControllerTest#test_bulk_create_should_show_errors [test/functional/issue_relations_controller_test.rb:270]:
"IssueRelation.count" didn't change by 3.
Expected: 5
  Actual: 6

rails test test/functional/issue_relations_controller_test.rb:269

.....

Finished in 585.005343s, 9.0649 runs/s, 41.6201 assertions/s.
5303 runs, 24348 assertions, 1 failures, 0 errors, 8 skips

Files

36361.patch (3.9 KB) 36361.patch

Yuichi HARADA, 2021-12-27 08:17

Actions

Copy link

Updated by Yuichi HARADA over 2 years ago

File 36361.patch 36361.patch added

IssueRelationsControllerTest#test_bulk_create_should_show_errors assumes that Setting.cross_project_issue_relations? is false.
When I ran the test, it was true, and although it should have been 3 records registered, 4 records could be registered, so this test failed.

As a result of the investigation, since Setting.cross_project_issue_relations = '1' was executed in source:trunk/test/unit/project_copy_test.rb@21326#L176, other It was affecting the test.

After creating a patch like the one below, the test was not failed.

diff --git a/test/unit/project_copy_test.rb b/test/unit/project_copy_test.rb
index 50cc1c9c98..91c45f2b10 100644
--- a/test/unit/project_copy_test.rb
+++ b/test/unit/project_copy_test.rb
@@ -173,38 +173,38 @@ class ProjectCopyTest < ActiveSupport::TestCase
   end

   test "#copy should copy issue relations" do
-    Setting.cross_project_issue_relations = '1'
-
-    second_issue = Issue.generate!(:status_id => 5,
-                                   :subject => "copy issue relation",
-                                   :tracker_id => 1,
-                                   :assigned_to_id => 2,
-                                   :project_id => @source_project.id)
-    source_relation = IssueRelation.create!(:issue_from => Issue.find(4),
+    with_settings :cross_project_issue_relations => '1' do
+      second_issue = Issue.generate!(:status_id => 5,
+                                     :subject => "copy issue relation",
+                                     :tracker_id => 1,
+                                     :assigned_to_id => 2,
+                                     :project_id => @source_project.id)
+      source_relation = IssueRelation.create!(:issue_from => Issue.find(4),
                                               :issue_to => second_issue,
                                               :relation_type => "relates")
-    source_relation_cross_project = IssueRelation.create!(:issue_from => Issue.find(1),
+      source_relation_cross_project = IssueRelation.create!(:issue_from => Issue.find(1),
                                                             :issue_to => second_issue,
                                                             :relation_type => "duplicates")

-    assert @project.copy(@source_project)
-    assert_equal @source_project.issues.count, @project.issues.count
-    copied_issue = @project.issues.find_by_subject("Issue on project 2") # Was #4
-    copied_second_issue = @project.issues.find_by_subject("copy issue relation")
-
-    # First issue with a relation on project
-    assert_equal 1, copied_issue.relations.size, "Relation not copied" 
-    copied_relation = copied_issue.relations.first
-    assert_equal "relates", copied_relation.relation_type
-    assert_equal copied_second_issue.id, copied_relation.issue_to_id
-    assert_not_equal source_relation.id, copied_relation.id
-
-    # Second issue with a cross project relation
-    assert_equal 2, copied_second_issue.relations.size, "Relation not copied" 
-    copied_relation = copied_second_issue.relations.find {|r| r.relation_type == 'duplicates'}
-    assert_equal "duplicates", copied_relation.relation_type
-    assert_equal 1, copied_relation.issue_from_id, "Cross project relation not kept" 
-    assert_not_equal source_relation_cross_project.id, copied_relation.id
+      assert @project.copy(@source_project)
+      assert_equal @source_project.issues.count, @project.issues.count
+      copied_issue = @project.issues.find_by_subject("Issue on project 2") # Was #4
+      copied_second_issue = @project.issues.find_by_subject("copy issue relation")
+
+      # First issue with a relation on project
+      assert_equal 1, copied_issue.relations.size, "Relation not copied" 
+      copied_relation = copied_issue.relations.first
+      assert_equal "relates", copied_relation.relation_type
+      assert_equal copied_second_issue.id, copied_relation.issue_to_id
+      assert_not_equal source_relation.id, copied_relation.id
+
+      # Second issue with a cross project relation
+      assert_equal 2, copied_second_issue.relations.size, "Relation not copied" 
+      copied_relation = copied_second_issue.relations.find {|r| r.relation_type == 'duplicates'}
+      assert_equal "duplicates", copied_relation.relation_type
+      assert_equal 1, copied_relation.issue_from_id, "Cross project relation not kept" 
+      assert_not_equal source_relation_cross_project.id, copied_relation.id
+    end
   end

   test "#copy should copy issue attachments" do

Actions

Copy link

Updated by Go MAEDA over 2 years ago

I think it is even better to use with_settings also in IssueRelationsControllerTest#test_bulk_create_should_show_errors for the following reasons:

We can clearly express that this test expects Setting.cross_project_issue_relations to be 0
It will never be affected by other tests

diff --git a/test/functional/issue_relations_controller_test.rb b/test/functional/issue_relations_controller_test.rb
index 099254979..d1bd9e60c 100644
--- a/test/functional/issue_relations_controller_test.rb
+++ b/test/functional/issue_relations_controller_test.rb
@@ -267,16 +267,18 @@ class IssueRelationsControllerTest < Redmine::ControllerTest
   end

   def test_bulk_create_should_show_errors
-    assert_difference 'IssueRelation.count', +3 do
-      post :create, :params => {
-        :issue_id => 1,
-        :relation => {
-          :issue_to_id => '1,2,3,4,5,7',
-          :relation_type => 'relates',
-          :delay => ''
-        }
-      },
-      :xhr => true
+    with_settings :cross_project_issue_relations => '0' do
+      assert_difference 'IssueRelation.count', +3 do
+        post :create, :params => {
+          :issue_id => 1,
+          :relation => {
+            :issue_to_id => '1,2,3,4,5,7',
+            :relation_type => 'relates',
+            :delay => ''
+          }
+        },
+        :xhr => true
+      end
     end

     assert_response :success

Actions

Copy link

Updated by Yuichi HARADA over 2 years ago

Go MAEDA wrote:

I think it is even better to use with_settings also in IssueRelationsControllerTest#test_bulk_create_should_show_errors for the following reasons:

We can clearly express that this test expects Setting.cross_project_issue_relations to be 0

It will never be affected by other tests

[...]

You are right. As you write, I also think should fix this test as well.

Actions

Copy link