SÅ‚awomir Grochowski <[email protected]> writes:

> Thank you all for the feedback. I completely understand the concerns
> about test suite maintenance and the risk of confusing arbitrary current
> behavior with the intended design.
>
> To address Ihor's point about explicitly marking this test, I would like
> to propose a few ways we could handle this.

Sorry for the late response. Just managed to get to this todo item, as
it took a while to process a number of patches that fixed bugs.
I tend to prioritize those over less urgent discussions.

>   1. Naming Convention 
>   We could rename the test to explicitly include the word "characterization"
>   (e.g. test-org-colview/characterization-insert-columns-keyword).
>   Running it: BTEST_RE='characterization' test-dirty. 
>   Within Emacs: M-x ert RET characterization RET. 
>
>   2. ERT Tags 
>   We could add a specific metadata tag, such as :tags '(characterization).  
>   Rnning it: M-x ert RET (tag characterization) RET or even (not (tag
>   characterization)) to exclude them. However, our Makefile does not have
>   a convenient way to filter by ERT tags. Running it from the CLI requires
>   bypassing make entirely with a verbose batch command like: emacs -Q
>   --batch ... --eval "(ert-run-tests-batch-and-exit '(tag
>   characterization))". But I haven't seen tags in our tests sutis. 

I am not 100% sure how valuable it would be to filter such tests.
In my mind, breaking of the "characterization" tests can happen in the
following scenario:

1. We make some bug fix or a change
2. We run make test
3. Observe tests broken
4. Regardless whether those tests are just for characterization or not,
   we need to look inside and see what exactly is being broken.
5. When looking inside the test, the main question becomes whether we
   should change the test to implement the original code change
   differently. For most tests, it will be that the code change should
   be done differently, or we should announce a breaking change, if it
   is justified. However, some tests will be marked as
   "characterization", so we will not need to worry as much and just
   free to update the test to follow the new slightly different
   implementation.

>   3. Docstring and "Pinning" Comments 
>   We could add a clear disclaimer in the docstring and a comment right 
>   above the assertion, explicitly stating that this is not a design
>   spec. For example:
>
>   "Record the current insertion position of a new #+COLUMNS keyword.
>
>   NOTE: This pins a known arbitrary behavior to prevent regressions. 
>   It does NOT represent the intended design spec.
>   See mailing list thread <...>"

In the above sequence, having an explicit comment on some test clause
(or on the whole test, in the docstring), will suffice. Adding extra
markings like test name or metadata seems excessive and just adds more
friction to creating new tests (because more conventions = more rules =
more things to keep in mind = harder contributions).

-- 
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>

Reply via email to