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

Already on GitHub? Sign in to your account

Merge and add tests related to 5215 #8184

merged 1 commit into from Nov 12, 2012


None yet
2 participants

vipulnsward commented Nov 12, 2012

Merge redundant tests and add new ones to has_and_belongs_to_many and has_many_through associations for null relation usage on new records.

@carlosantoniodasilva carlosantoniodasilva commented on the diff Nov 12, 2012

@@ -862,4 +856,15 @@ def klass.name; 'Foo'; end
def klass.name; 'Foo'; end
assert_deprecated { klass.has_and_belongs_to_many :posts, :delete_sql => 'lol' }
+ test "has and belongs to many associations on new records use null relations" do
+ projects = Developer.new.projects
+ assert_no_queries do
+ assert_equal [], projects
+ assert_equal [], projects.where(title: 'omg')
+ assert_equal [], projects.pluck(:title)
+ assert_equal 0, projects.count

carlosantoniodasilva Nov 12, 2012


I think you could add a line to test sum here as well, right?


vipulnsward Nov 12, 2012


Not possible on the model Project in question. Should I invert it to query on a developers relation instead? I guessed sum was trivial here so left it out.


carlosantoniodasilva Nov 12, 2012


No, that should be enough. I just thought it'd be worth, even a projects.sum(:id) should do :P. But that's fine already, thanks!

Looks good, thanks 👍

carlosantoniodasilva added a commit that referenced this pull request Nov 12, 2012

@carlosantoniodasilva carlosantoniodasilva merged commit 2a8b673 into rails:master Nov 12, 2012

@vipulnsward vipulnsward deleted the vipulnsward:add_merge_tests branch Feb 22, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment