Reconsider relation of usage examples and unit tests

[uliska]

This "issue" is meant for discussion. We should agree upon an approach here before starting any coding.
#85 brings up an issue I wanted to open anyway (just didn't find the opportunity until now).

I think we still have a problematic mix of purposes with the automated tests.
We collect all files from a usage-examples directory for the automated tests if they aren't explicitly excluded. Sounds like a good idea but it turns out that it is problematic because the usage examples we have so far (for GridLY and e.g. partial compilation) don't cover all situations.

Usage examples are part of the documentation, which is particularly obvious with the GridLY example. With such usage examples it is a natural approach to provide alternatives that the user can experiment with by uncommenting certain lines to activate alternative behaviour.

So it seems usage examples are similar but not identical with unit tests in their organization. Therefore I propose a different policy for usage examples and unit tests:

Libraries can (i.e. are strongly encouraged to) have

1. a (optionally hierarchic) usage-examples directory (usually on library top-level)
   All *.ly files in this (recursively) will be used for the auto-generated documentation
   (which hasn't been implemented yet. We'll have to think about the explicit in-/exclude options in that context separately)
2. a (flat) unit-tests directory
   All *.ly* files in this will be used for automated tests with Travis. These tests are like LilyPond's regression tests, and library maintainers are responsible for keeping the tests up to date and comprehensive. All relevant commands and constellations should be covered by tests. Usually it is a good idea to write one *.ily file containing the main includes and a bunch of smaller *.ly files covering individual tests or coherent groups of tests. This will also make possible failures point more directly to the cause.

In cases where the usage examples are appropriate as unit tests it is not necessary to duplicate them as test files. Instead they can simply be included through the .automated-tests-include approach.

---

I think this approach would reduce some collisions of concerns while still being straightforward and not imposing too much overhead or complexity on library maintainers.
And it shouldn't be complicated to change the implementation. The moment would be a good one because we a) do have a few examples we can use as proof-of-concept and b) we don't have too many examples that would have to be updated.

[uliska]

What I forgot: We should also find a way for the test files to define a range of LilyPond versions they are intended for.

So far there is the provision of header fields specifying minimal and maximal versions a "snippet" is known to work with. But that is something only LilyPond can determine when a library file is included. A Python script doesn't know about this when using a test file that includes such an incompatible file.

But I'd like to keep such compatibility information in one place exclusively, so I can think of another strategy:

• \loadModule tries to determine if the included file does have such compatibility header fields and checks the currently run version against them.
• If that test fails it throws a (ly:error) with a certain message.
• the testing script could interpret the return code of the lilypond script (but actually I don't know how Python will "see" the return value from a custom ly:error).
• if it can be determined as that kind of error the test is skipped (with a log message) but doesn't "fail".

[Cecca]

I agree with you that we should have the possibility to separate test and examples and I think that the chages to the current code will be minimal to introduce such a feature. However I think that we should keep the example files among the tested ones by default, not considering them as unit tests, but just to check that they compile on all supported versions.

We should also find a way for the test files to define a range of LilyPond versions they are intended for.

I don't have yet an opinion on how to implement this, but I agree that the information should be included only in one place.

As for capturing the error message of the lilypond script you can parse the standard error stream from python. For instance take a look at line 275 of automated_tests.py: you get a tuple with the standard output string and the standard error stream.

[uliska]

Am 28.02.2015 12:38, schrieb Matteo Ceccarello:

I agree with you that we should have the possibility to separate test
and examples and I think that the chages to the current code will be
minimal to introduce such a feature. However I think that we should
keep the example files among the tested ones by default, not
considering them as unit tests, but just to check that they compile on
all supported versions.

OK, I'll do the following then:

• keep the implementation of usage-example "harvesting" as it is
• add "unit-tests" to the harvested directories

However, I think .ily files should by default not be compiled. So one
doesn't have to explicitly exclude them.

Library maintainers are encouraged to cover all possible combinations
with tests. Everything that can reasonably be covered in the usage
examples is fine, for everything else they should provide explicit unit
tests.

We should also find a way for the test files to define a range of
LilyPond versions they are intended for.

I don't have yet an opinion on how to implement this, but I agree that
the information should be included only in one place.

As for capturing the error message of the lilypond script you can
parse the standard error stream from python. For instance take a look
at line 275
https://github.com/openlilylib/openlilylib/blob/18ce704de344e059e847b2061c8ff29a8f1acad7/test/automated_tests.py#L275
of |automated_tests.py|: you get a tuple with the standard output
string and the standard error stream.

Oh yes, I vaguely had this idea too but somehow didn't think further in
that direction.
Then it is possible like this:

• modify \loadModule so it looks for relevant header fields in the
  included file
• if it detects an incompatibility let the compilation fail with (ly:error)
• produce an error message that gives the regular user a clue about why
  it fails
  and lets the testing script make sense of it.
  The test would then not be considered as failed but as skipped.

The only thing we wouldn't be able to cover with this is a test that is
skipped by both test runs. But I think this is neglectable.

[Cecca]

OK.

However, I think .ily files should by default not be compiled. So one doesn't have to explicitly exclude them.

I'm fine with this, one can just include them by hand if necessary.

modify \loadModule so it looks for relevant header fields in the included file

Could you please wait a little before making these changes? I'm thinking of an alternative approach that should handle both this issue and the module version declartion.

[Cecca]

Oh, one more thing: how should we handle expected failures?

[uliska]

Oh, one more thing: how should we handle expected failures?

I'll think about this, but I have some ideas.

First of all, this should be rather rare cases and should not happen in
usage examples (at least not in their repository state (which is the
tested state) - they could however suggest to uncomment something to see
the failure).

We could either add a flag inside the file, which is determined in
is_runnable_file().
Or we could set up a file name convention so that, say, files ending
with -fail.ly are expected to fail.
Or we could make that case even more an exception and let them be
entered in a .automated-tests-fail file.

[Cecca]

As for versions (minimal, maximal, and module version) I was thinking about something like the following

\declareModule "gridly" \with {
  author = "Matteo Ceccarello"
  short-description = "Simple segmented grid framework"
  version = "0.4.0"
  minimal-lilypond-version = "2.18.2"
  maximal-lilypond-version = "2.19.15"
  description = "A brief descrtption of the library."
  category = "..."
  tags = "...."
}

in place of the current header declarations. This declareModule (or declareOllModule) can then perform version checks against the versino currently in use, declare the module version number in a standardized way and check that all relevant fields are present.

However, I don't know how much tooling there already is around openLilyLib header declarations and how they are used. So it may better to perform all these checks when loadModule is called.

[uliska]

Sounds good. I'll think about it when family duties are over later tonight.

Diese Nachricht wurde von meinem Android-Mobiltelefon mit K-9 Mail gesendet.

[uliska]

I think defining such a command is a good idea as it would avoid conflicts of purpose between header fields and module definition (e.g. it's not necessary anymore to use prefixes).

However, version conflicts should still be performed in \loadModule, but that's not a problem.

Existing code is mainly present in the doc-generation script - which has to be redone anyway, so that shouldn't be an issue.

Please not that minimal and maximal LilyPond versions are optional and should only be entered when there is a known limit (e.g. "this module won't work prior to 2.19.3")

[uliska]

@Cecca Please have a look at the general-interface branch ( Pull request #99). I have added several things to the public library interface, mainly \declareLibrary and \useLibrary

\declareLibrary is to be used in each library's __init__.ily, although that isn't enforced yet. The command accepts a differening display and internal name and a \with {} clause with library options. Options can be mandatory and type-checked. That is the command ensures that all mandatory options are declared and of the correct type.
Additionally "known" options are supported. These don't have to be present but are type-checked as well.

\useLibrary replaces \loadModule when used for loading a toplevel library (loading parts of a library hasn't been changed yet, but that has to come). (\loadModule has also been deprecated accordingly (that is, a warning is printed when the argument is a library and not a part of it).
The command accepts an (this time) optional \with {} clause that can be used to initialize the library with options (for example GridLY could provide an option to initialize the grid in this step already). Options passed to this clause are set using \setOption, which implicitly performs a check so only options can be used that are registered in the __init__ file.

The process of \useLibrary is:

• load the __init__ file
• set options passed to \useLibrary
• load __main__ file

This allows the main file to respond to the presence/values of any options set in \useLibrary.

What do you think?

I think the versioning should be done on library level while e.g. compatibility with LilyPond versions should (could) be on a lower per-module level.

[uliska]

(See the stylesheets library as an example of \declareLibrary

[Cecca]

Please have a look at the general-interface branch ( Pull request #99). I have added several things to the public library interface, mainly \declareLibrary and \useLibrary

Thank you Urs. Right now I'm busy, I'll have a look at this later this evening. Anyway, I linke this approach.

I think the versioning should be done on library level while e.g. compatibility with LilyPond versions should (could) be on a lower per-module level.

I don't quite get the difference between library level and module level.

[uliska]

This doesn't come as a surprise, as GridLY doesn't expose this difference.

But take ScholarLY as an example. This is a library, but it contains a number of modules / items. So far there is \annotate as a larger module (in its own subdirectory with several files) and a file diplomatic-line-breaks.ily. The latter is like a single tool in the toolbox. So we actually have three levels of distinction: library, module and tool, while I'm completely unsure about a coherent set of terms. But I think we should at least differentiate between libraries and modules/snippets/tools/toolboxes - whatever the best name is.

[Cecca]

OK, I understand. I think that having different modules of the same library support different lilypond versions can be potentially confusing for a user. Im my opinion if a module has different requirements on the LilyPond version with respect to the rest of the library, then it should be in a separate library.