commit fab4e18432bae0f11a9ed3d5c682c2442d2ef007 breaks pull request updates

[Thomas De Schampheleire]

On Wed, Jan 28, 2015 at 1:15 PM, Mads Kiilerich <mads at kiilerich.com> wrote:
> On 01/28/2015 12:12 PM, Thomas De Schampheleire wrote:
>>
>> Hi Mads,
>>
>>
>>> I can promise that your use case will keep working in the future if you
>>> add
>>> test coverage for it ;-)
>>
>> Is there any coverage already related to pull requests and possibly
>> pull request updates? I don't see any.
>> It would indeed be much better to have all this tested, but without a
>> good example I'm a bit at lost here.
>
>
> I think you are right that there not is any test coverage of this area.
>
> The existing tests use a unittest framework but it is not unittests at all.
> That makes it a bit painful to maintain. Still, it is better than nothing
> ... if we have tests.
>
> I guess a test would have to build a hg repo with the right structure,
> invoke Kallithea, and verify that the html output has the expected content.

In fact, I think it would be better if this could be tested without
having to verify html output. The test would create a repo with the
right structure, create a pull request using regular python code (I
mean, without sending http requests or the like) update the repo,
check the list of revisions available for update.

To put it differently: I think we should (in so much this is not
already the case; I haven't looked at all tests) decouple the testing
of the backend (the real code handling) from the frontend (html, js,
css, http, ajax, ...).

This may need some code restructuring though. For example,
PullrequestsController.show(), the method where this problem is
located, is one big method. It is not possible to read out the list of
revisions that are available for update from regular Python code, so
this is not testable. Instead, a helper method should be created that
returns the list of revisions available to update a given pull
request, which can then be called by the test.

>
> There is also
> https://bitbucket.org/conservancy/kallithea/pull-request/33/add-first-robotframework-pullrequest-test
> . It is even more high level and more "realistic"  but probably also slower
> ... and it doesn't work when I try to use it.

As far as I can see from the example test, this is testing from the
user perspective, like a robot indeed. But as I said above, I think we
should decouple the backend from the frontend testing. The robot
framework can be valuable to test some aspects of the frontend part,
but assuming there is good test coverage on the backend stuff, it
doesn't make sense to cover all scenarios in the robot framework.

Moreover, again based on the example tests, the test method is pretty
fragile, as it relies on certain html/css object names that could
easily change.

Best regards,
Thomas