Earl Chase <[email protected]> writes:
> ... What I don't understand is how are we supposed to ensure that
> our code doesn't break the user experience if we aren't allowed to test
> internal functions or undocumented behaviors? That's not a question for you
> obviously, that's a general question for the list.
We can test undocumented behavior, if we have evidence that many users
actually rely on it. In fact, it is a good idea to test it then (we
cannot put everything into the manual).
However, we should not put tests for arbitrary or ambiguous behavior
that has no reasoning behind it and just happens to be implemented one
or another way.
Here is what I wrote in testing/README file:
The test suite is designed to ensure stability of Org mode code base
as it evolves, being modified by numerous contributors. The tests are
usually designed aiming to ensure the *expected* Org mode behavior.
Thus, we should not add the tests that just mechanically check what
the current implementation does. In particular, we should avoid (or,
at least, explicitly mark) the tests that simply check for some
arbitrary behavior related to the details of the implementation; not
to the original design goals. The only exception is when users get
used to certain ways the code behaves by muscle memory.
As for making sure that user experience is not broken, we generally do
not need to test internal functions. Users do not generally interact with
Org mode using those internal functions. Instead, we should add tests to
the relevant public functions (that call the internal functions in
question under the hood). (Note that I haven't looked into the details
of the patch here yet, so my answer is rather generic)
> ... I am willing to keep working
> with you on this. I am willing to take critiques. However, I don't think it
> makes sense for me to review your patch on my thread. I started this thread
> fully expecting a code review. If you would like to do this refactor
> yourself, please let me know. I will cancel this thread so that you can
> start another one.
Just to make things clear, I asked Morgan to review some patches on the
list to practice, as he is willing to help with more advanced parts of
the maintenance duties. So, do not be too harsh on him. Morgan is still
learning things in this area. Of course, your reaction here is also
perfectly fine - you have all the rights to work on your patch and
expect the review.
--
Ihor Radchenko // yantar92,
Org mode maintainer,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>
