LocalBearTestHelper: Add `assertObjectsEqual` #4310

yash-nisar · 2017-06-02T14:15:45Z

Add assertObjectsEqual(..) method that compares individual
fields of the Result object and yields better, easy to understand
messages in case of an attribute mismatch.

Closes #4302

For short term contributors: we understand that getting your commits well
defined like we require is a hard task and takes some learning. If you
look to help without wanting to contribute long term there's no need
for you to learn this. Just drop us a message and we'll take care of brushing
up your stuff for merge!

Checklist

I read the commit guidelines and I've followed
them.
I ran coala over my code locally. (All commits have to pass
individually. It is not sufficient to have "fixup commits" on your PR,
our bot will still report the issues for the previous commit.) You will
likely receive a lot of bot comments and build failures if coala does not
pass on every single commit!

After you submit your pull request, DO NOT click the 'Update Branch' button.
When asked for a rebase, consult coala.io/rebase
instead.

Please consider helping us by reviewing other peoples pull requests as well:

pick up any PR at https://coala.io/review
review it (check https://coala.io/reviewing for more info)
if you are sure that it needs work, use cobot mark wip <URL> to get it out
of the review queue.

The more you review, the more your score will grow at coala.io and we will
review your PRs faster!

yash-nisar · 2017-06-02T14:18:29Z

status/blocked on coala_utils release which will include https://gitlab.com/coala/coala-utils/merge_requests/42

yash-nisar · 2017-06-02T14:18:38Z

CC @Makman2

yash-nisar · 2017-06-02T14:20:32Z

Note that the tests will fail because we still don't have the __compare_fields attribute which will only be added after https://gitlab.com/coala/coala-utils/merge_requests/42 is merged and a new version of coala_utils is released(maybe).

Makman2 · 2017-06-03T13:32:02Z

coalib/testing/LocalBearTestHelper.py

@@ -185,10 +201,11 @@ def check_results(self,
                                  create_tempfile=create_tempfile,
                                  tempfile_kwargs=tempfile_kwargs,
                                  settings=settings)
+        self.addTypeEqualityFunc(list, self.assertObjectsEqual)


This should be run somewhere in the beginning of a base test class or so, otherwise it takes effect only in check_validity

list doesn't use generate_eq or generate_repr, so this is not really working

But we're basically comparing list of Result objects, so whenever we pass lists to the assertEqual(...) for comparison, it will call the assertObjectsEqual(...) method IIUC. What I had initially done was put Result object instead of list since we wanna call that method whenever we're comparing Result objects, see below :
Snippet: self.addTypeEqualityFunc(list, self.assertObjectsEqual)
Output:

E AssertionError: 'mixed spaces and tabs' != 'mixed spaces and tab' E - mixed spaces and tabs E ? - E + mixed spaces and tab E : message_base mismatch

Snippet: self.addTypeEqualityFunc(Result, self.assertObjectsEqual)
Output:

E AssertionError: Lists differ: [<Result object(id=0xf33dc5cf898c48b4b7114cffd86e8172, origin='[490 chars]438>] != [<Result object(id=0xf211aef4abb24ecb8cc36beebbd848a8, origin='[489 chars]d30>] E E First differing element 0: E <Result object(id=0xf33dc5cf898c48b4b7114cffd86e8172, origin='[489 chars]a438> E <Result object(id=0xf211aef4abb24ecb8cc36beebbd848a8, origin='[488 chars]2d30> E E Diff is 2240 characters long. Set self.maxDiff to None to see it.

CC @Makman2

Should I create an __init__ method inside our LocalBearTestHelper class and put addTypeEqualityFunc(..) there ?

Another alternative that I've found is we pass the Result object to assertEqual which is something like(Error handling of empty lists, and so on will be done..) :

if not check_order: self.assertEqual(sorted(bear_output)[0], sorted(results)[0]) else: self.assertEqual(bear_output[0], results[0])

Then self.addTypeEqualityFunc(Result, self.assertObjectsEqual) snippet is justified IIUC.

Ouch haven't thought about that we pass results with lists... here we could just iterate over all passed results and compare them one by one with assertEqual. And we still register addTypeEqualityFunc with Result globally (or an ABC-type of classes using generate_eq and generate_ordering which would be the ideal solution).

Hm this is better than nothing, but ideally we would be able to have nicer outputs for collections... don't know something yet, maybe you also get some ideas how to make self.assertObjectsEqual working automatically everywhere in our tests, even those ones not testing only bears.

Ouch haven't thought about that we pass results with lists... here we could just iterate over all passed results and compare them one by one with assertEqual. And we still register addTypeEqualityFunc with Result globally (or an ABC-type of classes using generate_eq and generate_ordering which would be the ideal solution).

Aren't we doing this already in #4310 (comment) ?

Hm this is better than nothing, but ideally we would be able to have nicer outputs for collections... don't know something yet, maybe you also get some ideas how to make self.assertObjectsEqual working automatically everywhere in our tests, even those ones not testing only bears

Our main concern ATM is the not so useful output of bears so IMO we should get this basic functionality merged and those not testing only bears can be looked after later on ? What say ?

gitmate-bot · 2017-06-11T19:15:47Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

Makman2 · 2017-06-13T01:17:54Z

coalib/testing/LocalBearTestHelper.py

@@ -185,10 +201,11 @@ def check_results(self,
                                  create_tempfile=create_tempfile,
                                  tempfile_kwargs=tempfile_kwargs,
                                  settings=settings)
+        self.addTypeEqualityFunc(list, self.assertObjectsEqual)


We could actually use Iterable instead of only list, so we support any kind of iterable 👍

Or maybe for this case we should explicitly use assertObjectsEqual instead of registering it for tests. Makes it easier to understand^^

Makman2 · 2017-06-13T01:18:12Z

coalib/testing/LocalBearTestHelper.py

        else:
-            self.assertEqual(bear_output, results, msg=msg)
+            self.assertEqual(bear_output, results)


can we delete the msg variable above as we don't seem to use it any more?

yash-nisar · 2017-06-13T01:31:16Z

Will fix things suggested by the review comments in the next iteration and iterate over the list containing more than multiple results.

yash-nisar · 2017-06-19T11:23:24Z

Updated @Makman2

Makman2 · 2017-06-19T17:14:03Z

coalib/results/Result.py

@@ -25,7 +26,7 @@
                   'aspect',
                   'additional_info',
                   'debug_msg')
-class Result:
+class Result(Comparables):


We want to register Result to be a deriving type of Comparables inside the generate_eq/generate_ordering decorators, not here 👍

See https://docs.python.org/3.6/library/abc.html#abc.ABCMeta.register

Yeah, I just got to know the difference between the 2 ways of subclassing, the real one and the virtual one. I had actually referred to https://pymotw.com/3/abc/ which doesn't explicitly state the difference. My bad . :)

yash-nisar · 2017-06-22T13:07:32Z

How does this look ? @Makman2

yash-nisar · 2018-02-02T08:03:49Z

@Makman2 Ready for merge ?

gitmate-bot · 2018-02-02T08:03:51Z

Sorry @yash-nisar, you do not have the necessary permission levels to perform the action.

Makman2 · 2018-02-14T12:29:21Z

ack 14642d8

Makman2 · 2018-02-21T21:33:25Z

@gitmate-bot rebase

gitmate-bot · 2018-02-21T21:33:27Z

Hey! I'm GitMate.io! This pull request is being rebased automatically. Please DO NOT push while rebase is in progress or your changes would be lost permanently ⚠️

Add ``assertObjectsEqual(..)`` method that compares individual fields of the Result object and yields better, easy to understand messages in case of an attribute mismatch. Closes coala#4302

gitmate-bot · 2018-02-21T21:33:38Z

Automated rebase with GitMate.io was successful! 🎉

Makman2 · 2018-02-21T21:45:06Z

@gitmate-bot ff

gitmate-bot · 2018-02-21T21:45:09Z

Hey! I'm GitMate.io! This pull request is being fastforwarded automatically. Please DO NOT push while fastforward is in progress or your changes would be lost permanently ⚠️

gitmate-bot · 2018-02-21T21:45:20Z

Automated fastforward with GitMate.io was successful! 🎉

gitmate-bot added process/pending review size/S labels Jun 2, 2017

Makman2 reviewed Jun 3, 2017

View reviewed changes

Makman2 reviewed Jun 13, 2017

View reviewed changes

yash-nisar force-pushed the improve-testing-api branch from a0bd8c0 to 18eac8f Compare June 19, 2017 11:19

gitmate-bot added size/M review carefully! and removed size/S labels Jun 19, 2017

yash-nisar changed the title ~~LocalBearTestHelper: Add assertObjectsEqual~~ WIP LocalBearTestHelper: Add assertObjectsEqual Jun 19, 2017

yash-nisar force-pushed the improve-testing-api branch from 18eac8f to 8490074 Compare June 19, 2017 11:22

gitmate-bot added process/wip and removed process/pending review labels Jun 19, 2017

Makman2 reviewed Jun 19, 2017

View reviewed changes

yash-nisar force-pushed the improve-testing-api branch from 8490074 to 49ef411 Compare June 21, 2017 07:47

gitmate-bot added size/S and removed size/M labels Jun 21, 2017

yash-nisar force-pushed the improve-testing-api branch 2 times, most recently from 38dbea6 to 07bd68d Compare June 22, 2017 10:11

gitmate-bot added size/M and removed size/S labels Jun 22, 2017

yash-nisar force-pushed the improve-testing-api branch 2 times, most recently from b994c82 to 122dffa Compare June 22, 2017 13:06

gitmate-bot added size/S area/tests difficulty/medium and removed size/M labels Feb 2, 2018

gitmate-bot assigned yash-nisar Feb 2, 2018

gitmate-bot removed the process/approved The PR is approved and will be merged soon label Feb 2, 2018

gitmate-bot added process/approved The PR is approved and will be merged soon and removed process/pending review labels Feb 14, 2018

LocalBearTestHelper: Add assertObjectsEqual

31ad864

Add ``assertObjectsEqual(..)`` method that compares individual fields of the Result object and yields better, easy to understand messages in case of an attribute mismatch. Closes coala#4302

gitmate-bot force-pushed the improve-testing-api branch from 14642d8 to 31ad864 Compare February 21, 2018 21:33

gitmate-bot added process/pending review process/approved The PR is approved and will be merged soon and removed process/approved The PR is approved and will be merged soon process/pending review labels Feb 21, 2018

gitmate-bot added process/approved The PR is approved and will be merged soon and removed process/approved The PR is approved and will be merged soon labels Feb 21, 2018

gitmate-bot merged commit 31ad864 into coala:master Feb 21, 2018

gitmate-bot added the process/approved The PR is approved and will be merged soon label Feb 21, 2018

gitmate-bot removed the process/approved The PR is approved and will be merged soon label Jun 2, 2018

sangamcse mentioned this pull request Jun 10, 2018

BaseTestHelper.py: Add BaseTestHelper class #5496

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LocalBearTestHelper: Add `assertObjectsEqual` #4310

LocalBearTestHelper: Add `assertObjectsEqual` #4310

yash-nisar commented Jun 2, 2017 •

edited

Loading

yash-nisar commented Jun 2, 2017

yash-nisar commented Jun 2, 2017

yash-nisar commented Jun 2, 2017

Makman2 Jun 3, 2017

yash-nisar Jun 3, 2017

yash-nisar Jun 3, 2017 •

edited

Loading

yash-nisar Jun 3, 2017

Makman2 Jun 4, 2017

yash-nisar Jun 4, 2017

gitmate-bot commented Jun 11, 2017

Makman2 Jun 13, 2017

Makman2 Jun 13, 2017

Makman2 Jun 13, 2017

yash-nisar commented Jun 13, 2017

yash-nisar commented Jun 19, 2017

Makman2 Jun 19, 2017

Makman2 Jun 19, 2017

yash-nisar Jun 20, 2017

yash-nisar commented Jun 22, 2017

yash-nisar commented Feb 2, 2018

gitmate-bot commented Feb 2, 2018

Makman2 commented Feb 14, 2018

Makman2 commented Feb 21, 2018

gitmate-bot commented Feb 21, 2018

gitmate-bot commented Feb 21, 2018

Makman2 commented Feb 21, 2018

gitmate-bot commented Feb 21, 2018

gitmate-bot commented Feb 21, 2018

LocalBearTestHelper: Add assertObjectsEqual #4310

LocalBearTestHelper: Add assertObjectsEqual #4310

Conversation

yash-nisar commented Jun 2, 2017 • edited Loading

Checklist

yash-nisar commented Jun 2, 2017

yash-nisar commented Jun 2, 2017

yash-nisar commented Jun 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yash-nisar Jun 3, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gitmate-bot commented Jun 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yash-nisar commented Jun 13, 2017

yash-nisar commented Jun 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yash-nisar commented Jun 22, 2017

yash-nisar commented Feb 2, 2018

gitmate-bot commented Feb 2, 2018

Makman2 commented Feb 14, 2018

Makman2 commented Feb 21, 2018

gitmate-bot commented Feb 21, 2018

gitmate-bot commented Feb 21, 2018

Makman2 commented Feb 21, 2018

gitmate-bot commented Feb 21, 2018

gitmate-bot commented Feb 21, 2018

LocalBearTestHelper: Add `assertObjectsEqual` #4310

LocalBearTestHelper: Add `assertObjectsEqual` #4310

yash-nisar commented Jun 2, 2017 •

edited

Loading

yash-nisar Jun 3, 2017 •

edited

Loading