Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test errors after switching to MySQL #42

Closed
Yardboy opened this issue May 6, 2014 · 10 comments
Closed

Test errors after switching to MySQL #42

Yardboy opened this issue May 6, 2014 · 10 comments

Comments

@Yardboy
Copy link
Contributor

Yardboy commented May 6, 2014

Hiya

Cloned the repo, bundle installed and ran tests - all passed. Then I made the following changes:

  1. In Gemfile, removed sqlite3 gem, added mysql2 gem
  2. In database.yml, configured for mysql

Bundle installed again, ran all tests - 10 errors, as shown below. I started to look at a couple. I understand how to fix #4, but these "RecordInivalid expected" and "name already taken" errors have me stumped.

Do you have any idea why simply changing db's would cause these file folder errors?

Run options: --seed 30727

# Running:

...............F..F....F..........E....F...F.....E........E....EE....

Finished in 4.411355s, 15.6415 runs/s, 55.9919 assertions/s.

  1) Failure:
FolderTest#test_copy_a_folder [/home/cayce/workspace/boxroom/test/unit/folder_test.rb:127]:
ActiveRecord::RecordInvalid expected but nothing was raised.


  2) Failure:
FolderTest#test_move_a_folder [/home/cayce/workspace/boxroom/test/unit/folder_test.rb:158]:
--- expected
+++ actual
@@ -1 +1 @@
-#<Folder id: 142, name: "Root folder", parent_id: nil, created_at: "2014-05-06 01:10:28", updated_at: "2014-05-06 01:10:28">
+#<Folder id: 127, name: "Root folder", parent_id: nil, created_at: "2014-05-06 01:10:27", updated_at: "2014-05-06 01:10:27">



  3) Failure:
FolderTest#test_whether_a_folder_has_children_or_not [/home/cayce/workspace/boxroom/test/unit/folder_test.rb:184]:
--- expected
+++ actual
@@ -1 +1 @@
-#<Folder id: 153, name: "Root folder", parent_id: nil, created_at: "2014-05-06 01:10:28", updated_at: "2014-05-06 01:10:28">
+#<Folder id: 127, name: "Root folder", parent_id: nil, created_at: "2014-05-06 01:10:27", updated_at: "2014-05-06 01:10:27">



  4) Error:
ShareLinkTest#test_emails_is_not_longer_than_256_characters:
ActiveRecord::StatementInvalid: Mysql2::Error: Data too long for column 'emails' at row 1: UPDATE `share_links` SET `emails` = 'email@domain.com, another-email@domain.com, email@domain.com, another-email@domain.com, email@domain.com, another-email@domain.com, email@domain.com, another-email@domain.com, email@domain.com, another-email@domain.com, email@domain.com, another@domain.com', `link_token` = '96ef2551102ac909797d', `updated_at` = '2014-05-06 01:10:28' WHERE `share_links`.`id` = 26
    test/unit/share_link_test.rb:17:in `block in <class:ShareLinkTest>'


  5) Failure:
UserFileTest#test_attachment_file_name_is_unique [/home/cayce/workspace/boxroom/test/unit/user_file_test.rb:55]:
Failed assertion, no message given.


  6) Failure:
UserFileTest#test_copy_a_file [/home/cayce/workspace/boxroom/test/unit/user_file_test.rb:85]:
ActiveRecord::RecordInvalid expected but nothing was raised.


  7) Error:
UserTest#test_cannot_delete_admin_user:
ActiveRecord::RecordInvalid: Validation failed: Name has already been taken
    app/models/user.rb:105:in `create_root_folder_and_admins_group'
    test/unit/user_test.rb:88:in `block in <class:UserTest>'


  8) Error:
UserTest#test_password_is_valid:
ActiveRecord::RecordInvalid: Validation failed: Name has already been taken
    app/models/user.rb:105:in `create_root_folder_and_admins_group'
    test/unit/user_test.rb:10:in `block in <class:UserTest>'


  9) Error:
UserTest#test_user_permissions:
ActiveRecord::RecordInvalid: Validation failed: Name has already been taken
    app/models/user.rb:105:in `create_root_folder_and_admins_group'
    test/unit/user_test.rb:96:in `block in <class:UserTest>'


 10) Error:
UserTest#test_whether_a_user_is_member_of_admins_or_not:
ActiveRecord::RecordInvalid: Validation failed: Name has already been taken
    app/models/user.rb:105:in `create_root_folder_and_admins_group'
    test/unit/user_test.rb:167:in `block in <class:UserTest>'

69 runs, 247 assertions, 5 failures, 5 errors, 0 skips
@mischa78
Copy link
Owner

mischa78 commented May 6, 2014

I'm getting the same problem. I will investigate what causes this problem and let you know.

@Yardboy
Copy link
Contributor Author

Yardboy commented May 6, 2014

I'm having a look as well. Will let you know if I get any fixed and get you a pull request.

@mischa78
Copy link
Owner

mischa78 commented May 6, 2014

I'm getting less errors than you though. Seems some validations are not running. Very strange.

Run options: --seed 42811

# Running:

...............F..F....F..........E....F...F...................F.....

Finished in 1.883689s, 36.6303 runs/s, 134.3109 assertions/s.

  1) Failure:
FolderTest#test_copy_a_folder [/Users/mischa/Sites/boxroom/test/unit/folder_test.rb:127]:
ActiveRecord::RecordInvalid expected but nothing was raised.


  2) Failure:
FolderTest#test_move_a_folder [/Users/mischa/Sites/boxroom/test/unit/folder_test.rb:158]:
--- expected
+++ actual
@@ -1 +1 @@
-#<Folder id: 301, name: "Root folder", parent_id: nil, created_at: "2014-05-06 12:41:02", updated_at: "2014-05-06 12:41:02">
+#<Folder id: 286, name: "Root folder", parent_id: nil, created_at: "2014-05-06 12:41:02", updated_at: "2014-05-06 12:41:02">



  3) Failure:
FolderTest#test_whether_a_folder_has_children_or_not [/Users/mischa/Sites/boxroom/test/unit/folder_test.rb:184]:
--- expected
+++ actual
@@ -1 +1 @@
-#<Folder id: 312, name: "Root folder", parent_id: nil, created_at: "2014-05-06 12:41:02", updated_at: "2014-05-06 12:41:02">
+#<Folder id: 286, name: "Root folder", parent_id: nil, created_at: "2014-05-06 12:41:02", updated_at: "2014-05-06 12:41:02">



  4) Error:
ShareLinkTest#test_emails_is_not_longer_than_256_characters:
ActiveRecord::StatementInvalid: Mysql2::Error: Data too long for column 'emails' at row 1: UPDATE `share_links` SET `emails` = 'email@domain.com, another-email@domain.com, email@domain.com, another-email@domain.com, email@domain.com, another-email@domain.com, email@domain.com, another-email@domain.com, email@domain.com, another-email@domain.com, email@domain.com, another@domain.com', `link_token` = '99a1e37c5942f11c2bbe', `updated_at` = '2014-05-06 12:41:02' WHERE `share_links`.`id` = 62
    test/unit/share_link_test.rb:17:in `block in <class:ShareLinkTest>'


  5) Failure:
UserFileTest#test_attachment_file_name_is_unique [/Users/mischa/Sites/boxroom/test/unit/user_file_test.rb:55]:
Failed assertion, no message given.


  6) Failure:
UserFileTest#test_copy_a_file [/Users/mischa/Sites/boxroom/test/unit/user_file_test.rb:85]:
ActiveRecord::RecordInvalid expected but nothing was raised.


  7) Failure:
UserTest#test_user_permissions [/Users/mischa/Sites/boxroom/test/unit/user_test.rb:101]:
Failed assertion, no message given.

69 runs, 253 assertions, 6 failures, 1 errors, 0 skips

@Yardboy
Copy link
Contributor Author

Yardboy commented May 6, 2014

So, the issue with all of these errors except for #4 (the field length issue) appears to be related to this code in folder.rb:

83 def self.root
84   @root_folder ||= find_by_name_and_parent_id('Root folder', nil)
85 end

You are storing the root folder, once it is retrieved, presumably so that you don't hit the database the next time you access Folder.root. All fine and good in the working app.

However, the test db gets rolled back for each test, so the root folder is getting recreated each time FactoryGirl.create(:folder) is called. Apparently, SQLite3 resets the auto-increment counter on a table when a transaction is rolled back, but MySQL (and PostgreSQL, for that matter) does not. Because of this, each new creation of the root folder under MySQL gets a new ID number. The @root_folder instance variable never gets reset, it still refers to the first root folder that was created, including the original ID number, so in these tests you never get a match between the parent (current root folder) and the @root_folder value returned by the Folder#root method (original root folder).

This is proven out by adding the following at line 148 of folder_test.rb:

148 raise [Folder.where(:name => 'Root folder'), Folder.root].inspect

... and then running your tests. Look at the "move a folder" test results. You'll see a trace listing two folder objects with different ID's. This indicates that Folder.root holds an object that no longer resides in the db, otherwise they'd match.

Changing the code as such results in these test errors going away, further proving that this is the issue:

83 def self.root
84   find_by_name_and_parent_id('Root folder', nil)
85 end

However, that's not really a solution - the issue isn't really with the code, it's with the way the testing runs under MySQL. I'm not sure if this is the cleanest fix for this, but here's what I did.

I added the following class method to Folder:

def self.clear_root!
  @root_folder = nil
end

And I added a setup step in the folder and userfile tests to call that method before each test to insure that the root_folder is valid for that test.

def setup
  Folder.clear_root!
end

Sending you a pull request for all this. I also fixed the email too long issue on the ShareLink. Rails migrations on MySQL create string fields as VARCHAR(255), so I bumped the validation down from 256 to 255.

@Yardboy Yardboy mentioned this issue May 6, 2014
@Yardboy
Copy link
Contributor Author

Yardboy commented May 6, 2014

I got less errors here at work as well - I'm not sure what the other three were about, but I will double-check them on home computer (where I was getting the original 10) tonight.

@Yardboy
Copy link
Contributor Author

Yardboy commented May 6, 2014

Looking at the error differences, I believe that somewhere towards the end of debugging last night I may have run the install in TEST, which would account for those three additional "already taken" errors. Sorry about that.

@mischa78
Copy link
Owner

mischa78 commented May 6, 2014

Since this is only used in test, instead of adding a clear_root! class method to the Folder class, I would prefer implementing setup like this:

def setup
  Folder.instance_variable_set('@root_folder', nil)
end

@Yardboy
Copy link
Contributor Author

Yardboy commented May 6, 2014

Fair enough! Give me a few minutes and I'll update my pull branch.

@Yardboy
Copy link
Contributor Author

Yardboy commented May 6, 2014

Done! I added a test_helper method to execute code as above, and called that helper from folder and userfile tests.

@mischa78
Copy link
Owner

mischa78 commented May 6, 2014

Thanks for noticing this, finding out what was wrong and fixing it. I did one more commit to reflect the change from 256 char to 255 char in the UI as well: 4508f02

@mischa78 mischa78 closed this as completed May 6, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants