Defect #29883

AttachmentsVisibilityTest and Redmine::AttachmentFieldFormatTest fail randomly due to uninitialized User.current

Added by Gilad Shanan 17 days ago. Updated 14 days ago.

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

0%

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

Description

It looks like test/functional/attachments_visibility_test.rb has an unspecified dependency on the value of User.current. To reproduce:

bin/rails test test/integration/api_test/authentication_test.rb test/functional/attachments_visibility_test.rb --seed 53775

=>
Failure:
AttachmentsVisibilityTest#test_attachment_should_be_visible [/Users/gilad/Workspace/redmine/test/functional/attachments_visibility_test.rb:43]:
Expected response to be a <2XX: success>, but was a <403: Forbidden>
Environment:
  Redmine version                3.4.6.devel
  Ruby version                   2.4.1-p111 (2017-03-22) [x86_64-darwin17]
  Rails version                  5.2.1
  Environment                    production
  Database adapter               Mysql2
  Mailer queue                   ActiveJob::QueueAdapters::AsyncAdapter
  Mailer delivery                smtp
Redmine plugins:
  no plugin installed

There are several tests that change User.current and don't reset the value to nil during teardown. In this case the culprit was the authorization_test.rb but I believe there are other tests that could potentially cause the same problem. I suggest adding `User.current = nil` to `setup` in the failing attachments_visibility_test.rb, and also adding the same to `teardown` in authorization_test.rb.

0001-Fix-spurious-spec-failure.diff Magnifier (1.45 KB) Gilad Shanan, 2018-10-30 19:48

0001-Fix-spurious-spec-failures.patch Magnifier (2.16 KB) Gilad Shanan, 2018-10-30 22:53

Associated revisions

Revision 17616
Added by Go MAEDA 14 days ago

Fix that the following tests fail randomly due to uninitialized User.current (#29853):

  • AttachmentsVisibilityTest#test_attachment_should_be_visible *
    Redmine::AttachmentFieldFormatTest#test_should_accept_a_hash_with_upload_on_create *
    Redmine::AttachmentFieldFormatTest#test_should_replace_attachment_on_update

Patch by Gilad Shanan.

History

#1 Updated by Gilad Shanan 17 days ago

Another related spec failure can be reproduced like this:

bin/rails test test/integration/api_test/authentication_test.rb test/unit/lib/redmine/field_format/attachment_format_test.rb --seed 53775

=>
Failure:
Redmine::AttachmentFieldFormatTest#test_should_accept_a_hash_with_upload_on_create [/Users/gilad/Workspace/redmine/test/unit/lib/redmine/field_format/attachment_format_test.rb:45]:
--- expected
+++ actual
@@ -1 +1 @@
-#<CustomValue id: 284, customized_type: "Principal", customized_id: 133, custom_field_id: 46, value: "69">
+nil

Failure:
Redmine::AttachmentFieldFormatTest#test_should_replace_attachment_on_update [/Users/gilad/Workspace/redmine/test/unit/lib/redmine/field_format/attachment_format_test.rb:123]:
"Attachment.count" didn't change by 0.
Expected: 21
  Actual: 22

I've update the patch to address this as well.

#2 Updated by Go MAEDA 17 days ago

  • Target version set to 4.1.0

I know the test fails from time to time but I didn't know the reason. Thank you for investigating that.

The patch looks good. Setting target version to 4.1.0.

#3 Updated by Go MAEDA 14 days ago

  • Subject changed from AttachmentsVisibilityTest missing setup step User.current = nil to AttachmentsVisibilityTest and Redmine::AttachmentFieldFormatTest fail randomly due to uninitialized User.current
  • Status changed from New to Closed
  • Assignee set to Go MAEDA
  • Target version changed from 4.1.0 to 4.0.0
  • Resolution set to Fixed

Committed. Thank you for fixing those hard to find issues.

Also available in: Atom PDF