Code review comment for lp:~paulliu/unity-scope-click/showratings

Revision history for this message
dobey (dobey) wrote :

48 + --pkg rest-0.7 \

I thought we agreed to not use librest, but just use soup for everything?

Also, the test doesn't seem to actually test anything, as I read it. It's fake data in a JSON dict, and you parse it as such. We don't need to write unit tests for json-glib in our code. It's also a bit weird to define a test method inside a fake class, and simply have the actual test method call that method, doing the assertion inside the fake class. Fake classes should really only be used to override methods which require network or dbus access to other daemons, to prevent such access and isolate the tests. Fake data should probably be placed in external files for use in the tests, or at the very least, encoded in the fake-data.vala source file. And the actual test and assertion should happen within the test function itself, not inside a set of methods in another class.

review: Needs Fixing

« Back to merge proposal