[Scummvm-devel] Event Recorder Dependencies Breaking Tests

[Eugene Sandulenko]

That's great! Thank you!


Eugene


On 6 July 2013 07:21, D G Turner <d.g.turner at ntlworld.com> wrote:

> Eugene,
>   I have looked at the reason for the "make test" failure since the
>   Event Recorder was merged. This is due to the new and undesired
>   dependency between the common and GUI code introduced by this.
>
>   Though removing all of this requirement is a more complex design
>   question, I have worked out that it is possible to restore the
>   tests functionality for now.
>
>   This will require the attached patch to common/system.cpp to be
>   applied. It adds #ifdef ENABLE_EVENTRECORDER guards which are
>   currently not present in this file, but are very similar to those
>   present in various other files including base/main.cpp.
>
>   This will then truly remove the dependency when the event recorder
>   is disabled, rather than leaving some linkages in system.cpp.
>
>   I can then fix the tests on buildbot by adding "--disable-
>   eventrecorder" to the configure options passed to the debian-x86
>   native build.
>
>   As this is a fairly trivial patch, I didn't think it warranted a
>   Pull Request, but I didn't want to cause another argument with the
>   Core Team if I applied this fix directly, so am sending it to the
>   list for comment and review.
>
>   I am aware that this could be considered a "hack", though my view is
>   that those guards should be present in that file in any case and this
>   solution has the advantage of restoring the tests functionality now,
>   rather that leaving these broken (which would leave the common code
>   open to developing bugs which are not picked up until much later).
>
>   Any views on this? If there are no objections, I will apply this
>   patch in the next few days and make the requisite change to buildbot.
> Thanks,
> David Turner
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.scummvm.org/pipermail/scummvm-devel/attachments/20130706/ef767e5c/attachment.html>