Add math test

[nikolai-b]

After rails/arel#449 was merged math can be done on these
nodes, adding a spec.

Copied over from rails/arel#524

[kamipo]

Since Arel is private API, can we test this behavior from the public API?

[matthewd]

I think we can follow existing precedent for Arel tests. While it is private API, it exists to provide a clean layer to build AR on top of, so IMO it makes sense for it to have thorough unit tests.

[kamipo]

I see. So can we more exercise another expressions too (maximum, minimum, and average)?

[nikolai-b]

They are all defined

rails/activerecord/lib/arel/nodes/function.rb

Lines 34 to 40 in 989b1cb

	%w{
	Sum
	Exists
	Max
	Min
	Avg
	}.each do |name|

and as far as I can see they don't have any tests: https://github.com/rails/rails/tree/master/activerecord/test/cases/arel/nodes .

Would you like me to add some?

[kamipo]

For now few tests exists in the file:

rails/activerecord/test/cases/arel/attributes/attribute_test.rb

Lines 313 to 389 in ac93e7b

	describe "#average" do
	it "should create a AVG node" do
	relation = Table.new(:users)
	relation[:id].average.must_be_kind_of Nodes::Avg
	end
	it "should generate the proper SQL" do
	relation = Table.new(:users)
	mgr = relation.project relation[:id].average
	mgr.to_sql.must_be_like %{
	SELECT AVG("users"."id")
	FROM "users"
	}
	end
	end
	describe "#maximum" do
	it "should create a MAX node" do
	relation = Table.new(:users)
	relation[:id].maximum.must_be_kind_of Nodes::Max
	end
	it "should generate proper SQL" do
	relation = Table.new(:users)
	mgr = relation.project relation[:id].maximum
	mgr.to_sql.must_be_like %{
	SELECT MAX("users"."id")
	FROM "users"
	}
	end
	end
	describe "#minimum" do
	it "should create a Min node" do
	relation = Table.new(:users)
	relation[:id].minimum.must_be_kind_of Nodes::Min
	end
	it "should generate proper SQL" do
	relation = Table.new(:users)
	mgr = relation.project relation[:id].minimum
	mgr.to_sql.must_be_like %{
	SELECT MIN("users"."id")
	FROM "users"
	}
	end
	end
	describe "#sum" do
	it "should create a SUM node" do
	relation = Table.new(:users)
	relation[:id].sum.must_be_kind_of Nodes::Sum
	end
	it "should generate the proper SQL" do
	relation = Table.new(:users)
	mgr = relation.project relation[:id].sum
	mgr.to_sql.must_be_like %{
	SELECT SUM("users"."id")
	FROM "users"
	}
	end
	end
	describe "#count" do
	it "should return a count node" do
	relation = Table.new(:users)
	relation[:id].count.must_be_kind_of Nodes::Count
	end
	it "should take a distinct param" do
	relation = Table.new(:users)
	count = relation[:id].count(nil)
	count.must_be_kind_of Nodes::Count
	count.distinct.must_be_nil
	end
	end

How about adding math_test.rb instead of adding one .sum - 2 test case?
Not only sum and count but also attributes (e.g. table[:id]) can be mathmatically operated, but not tested.

[vedant1811]

Why is this still open?

[nikolai-b]

@vedant1811 it is adding some tests for an untested part of the code. I will add more tests as @kamipo suggests but I have not had time at the moment.

[nikolai-b]

@kamipo I've added tests for the math operators. Is that what you were suggesting?

[kamipo]

Is there any reason not to include * and / in the list?

[nikolai-b]

Yes, they have a slightly different SQL signature https://github.com/rails/rails/pull/32724/files/2bf08c892b1c14d5c4c433e791017dd8d499b21e#diff-e79446cf78f4dc85660b8a7c9bbec429R8

E.g.

(AVG("users"."id") + 2)

vs

AVG("users"."id") * 2

[kamipo]

table[:id] is not table node, but attribute node.

rails/activerecord/lib/arel/table.rb

Lines 81 to 83 in df850b8

	def [](name)
	::Arel::Attribute.new self, name
	end

rails/activerecord/lib/arel/attributes/attribute.rb

Lines 5 to 10 in df850b8

	class Attribute < Struct.new :relation, :name
	include Arel::Expressions
	include Arel::Predications
	include Arel::AliasPredication
	include Arel::OrderPredications
	include Arel::Math

[nikolai-b]

Thank you, I've updated it 765a04a78c

[kamipo]

Looks good to me. Can you squash your commits into one?