pushing patches despite requests for a test

Jan Holesovsky kendy at collabora.com
Mon Jul 27 02:29:29 PDT 2015


Hi Moggi, all,

Markus Mohrhard píše v So 25. 07. 2015 v 02:10 +0200:


> Additionally I consider the "it's insane to rejecting this on the
> grounds of unit tests" a quite direct personal attack. Normally I
> would have tried to talk directly with someone who pushes such a patch
> but after such a statement which is clearly targeted at me I prefer
> that the conversation happens in the open.

I see - sorry about that :-(  I don't think Tomaž really meant it the
way it sounded in the end; probably best if you both can join the ESC
this week?

> I stand by my decision not to accept any chart patches that have no
> unit test (currently pure dialog code is excluded as there is no sane
> way to test it).

Actually I believe we have more areas that have this rule, just we don't
have them listed anywhere; so I've extended the

https://wiki.documentfoundation.org/Development/Unit_Tests

with two things:

* list of areas that have "No fix without a unit test" rule (Miklos, I
  believe you have the same rule in the import/export filters - can you
  please specify there)?

  Everyone - please add more as you see fit + feel responsible for

* explanation of how to actually unit-test stuff

  I've added many types I was able to think of, but if you know more,
  please do add too.

> The correct thing would have been to talk to me. I would have helped
> with the XShape tests as I did for anyone else who has needed some
> help with tests.

The author of the patch apparently asked for help there, but I'm afraid
he did not get the "do this and that" kind of info that he needed
there :-(  I've now extended the wiki page with various test scenarios,
hopefully that could be used as a reference in such cases going forward;
please extend it as you see fit.

The unfortunate part was that a casual contributor got in the way
between two experienced developers, and his patch was reverted because
of things that he could not really influence.

I am concerned that he was not even notified that his patch was reverted
- the revert was not done via gerrit which would notify him; but
hopefully there was some personal mail I (obviously) don't know about.

So - in addition to re-iterating the areas that always need unit tests,
I'd also like to re-iterate that no revert should be done without
notifying the author.

Either way - I hope we have some great outcomes in the end:

* the new getXShapeDumpXmlDoc() / assertXPath() way of testing the
  XShapes results (would be great to move it to a more general place so
  that it can be used in Impress too)

* extended Unit Tests wiki page to be more usable

* some rules to re-iterate / re-check in the ESC

Let's discuss on Thursday - but I hope all this will improve the strive
for quality even further.

All the best,
Kendy



More information about the LibreOffice mailing list