maggienj / ActiveData

Provide high speed filtering and aggregation over data
Mozilla Public License 2.0
0 stars 0 forks source link

Fix test_max_object_on_value #22

Closed klahnakoski closed 7 years ago

klahnakoski commented 7 years ago

This test will help us practice the formal process of fixing a bug.

maggienj commented 7 years ago

Tried with this change to test... "expecting_list" section alone...

"select": [ {"name":"max" , "value": "a", "aggregate": "max"} ]

expecting_list has passed the expectation after making this change to test.

maggienj commented 7 years ago

Brought back the other expectations such as "expecting_table" and "expecting_cube" . The test appears to be passing now.

maggienj commented 7 years ago

In this line, "select": [ {"name":"max" , "value": "a", "aggregate": "max"} ]

Trying it with "value": "." instead of "a" .

For "." , it errors. For "a" it works.

In the debugger window... all object names are "." , so it should have worked for "." but, not sure why it doesn't work for "."

maggienj commented 7 years ago

Changes done for this test to pass are : (1) self.utils.execute_es_tests(test, tjson=False)

(2) "select": [ {"name":"max", "value": "a", "aggregate": "max"} ] Added the header "max".

For this test to pass, we could have added the header name "max" or removed the header name from expectation list... I just tried to add the header name "max" as the "expectation_list" was expecting the value with the header of "max" .

Not sure, if the "expectation_list" should be modified without the header.... that's why made this change to make this test to pass.

Test Passed. Committed and Pushed.

maggienj commented 7 years ago

In Json terms, this should be called as "name" property with a value of "max".

maggienj commented 7 years ago

Just figured out that the "expection_list" need not be modified in the "Test definition". Because, we are expecting an object , as per the name of the test which is "test_max_object_on_value"

So, adding the header or populating the name property with a value of "max" appears to be right to fix this test.

klahnakoski commented 7 years ago

You need not describe the changes you have done to get the test to pass: Commit and push so I can see those changes.

maggienj commented 7 years ago

Committed and pushed , as soon as the test got passed.

maggienj commented 7 years ago

The description is mostly for me... because at times, even though it worked... I didnt know why it worked or why it passed fine.
It could have been that something else could have been changed for the test to pass... namely the "expectation_list" versus the "current change", since the problem was with the "test definition" in this scenario.

That's why wrote the description...

maggienj commented 7 years ago

oops.... looks like only one file was committed and pushed. Adding the other file now...to commit and push.

maggienj commented 7 years ago

well, no other file is showing up now to commit and push.
( it did show one file earlier.... now, it isn't showing anything to commit or push )

maggienj commented 7 years ago

fixed.