Re: Context regression tests (issue 348760043 by address@hidden)

[Carl . D . Sorensen]

On 2018/05/03 02:48:10, Dan Eble wrote:
> I would appreciate a close review of these tests by at least one of
the
> long-time contributors or pro users.  Contexts are a central part of
LilyPond
> and if I've misjudged how any of these cases should work, I don't want
it to
> slip by.  Thanks.

> If you have suggestions on a better approach to testing, I'm all ears.

> James, please leave this in review until there is feedback.

This is a very impressive test suite.  You have done a great job of
considering possibilities and developing code to demonstrate them.

I'm a little bit surprised by the nature of this testing, however.  I
have always assumed that context id's should be unique, and that if one
created two contexts of the same type with the same id, the results
would be undefined.  You've looked at how the code works and ferreted
out the behaviors you are testing in this set of tests, but I don't
believe that the behavior you have identified should be
contracturally-defined behavior.

In my opinion, the most ideal result when one tries to create a new
context with the same type and id of an existing context would be to
generate an error, something like "Error: duplicate Staff with ID of A".

If we promise the behavior that your regression tests demonstrate, then
we have developed an official scope for context IDs, and most LilyPond
constructs do not have scope.

But this is just my initial opinion, and I am certainly open to other
arguments.

Thanks,

Carl


https://codereview.appspot.com/348760043/