Bug 1460035

PR submitted to GitHub:

    https://github.com/beakerlib/beakerlib/pull/5

When the libraries concept was created we agreed on mandatory initialization function not only for initialization but also for sanity checking that it is really a library.
This is documented and it is mandatory part of library's API.
I don't see any advantage of removing that. As the function can be trivial it is also trivial to place it there.

(In reply to Dalibor Pospíšil from comment #2)
> When the libraries concept was created we agreed on mandatory initialization
> function not only for initialization but also for sanity checking that it is
> really a library.

Anyway who does 'sanity check that it is really a library'?  Certainly not rlImport (and I don't think it should).  It simply logs error that the initialization handler failed, which is not true because the handler does not exist.

I don't think it's good design to have 2 places to say "i am a Beakerlib library".  We already do it in Makefile and that is much more formal anyway.


> This is documented and it is mandatory part of library's API.
> I don't see any advantage of removing that. As the function can be trivial
> it is also trivial to place it there.

As I said, it clutters code and documentation.   If I have library with 1 public function, I can't because I always have to add LibraryLoaded (which I never need.)

~

Also, another way of looking at it: this concept (mandatory API) does not come in any other languages (at least those I can think of).  What is it so special about beakerlib to justify having it?

Please re-consider this one more time.

(In reply to Alois Mahdal from comment #3)
> Anyway who does 'sanity check that it is really a library'?  Certainly not
> rlImport (and I don't think it should).  It simply logs error that the
> initialization handler failed, which is not true because the handler does
> not exist.
rlImport checks (by calling) then <prefix>LibraryLoaded function that the defined prefix matches the function prefix.
> 
> I don't think it's good design to have 2 places to say "i am a Beakerlib
> library".  We already do it in Makefile and that is much more formal anyway.
> 
> Also, another way of looking at it: this concept (mandatory API) does not
> come in any other languages (at least those I can think of).  What is it so
> special about beakerlib to justify having it?
Beakerlib is not a language it is a test framework, or a test library. And as such it has its own rules, this is one of them.

Still I'm missing opinion of other maintainers. My opinion is to reject this issue. If others have opposite opinion we can discuss further.

Moreover a library is not something you write from scratch every day so I think it is not that big deal. Also beaker-wizard should prepare everything for you.

The purpose of the function is not just to say 'I'm a library, folks' but at the first place to indicate whether the library has been loaded correctly or not. The very minimal check is that the function itself is defined, more sophisticated checks could include checking of the 'real' functions, variables, values, whatsoever.

Thus, I don't support the change.

Well we consider this as unjustified RFE, thus closing this bug.

> Beakerlib is not a language it is a test framework, or a test library. And as
> such it has its own rules, this is one of them.

I recommend this question on SO:

    https://stackoverflow.com/questions/4099975

Beakerlib is certainly not a framework.  Frameworks hold the flow control and just pass it to modules in order to implement business-specific behaviors.  That's why they *need* functions (or similar elements) to be implemented by their "subordinates".  (E.g. beah is a framework -- that's why it requires `make run` target.)

So, point of rlImport is not to pass control to the lib.sh but to have it define functions and variables.  You don't *need* `*LibraryLoaded` for that.

~

> The purpose of the function is not just to say 'I'm a library, folks' but at
> the first place to indicate whether the library has been loaded correctly or
> not. The very minimal check is that the function itself is defined, more
> sophisticated checks could include checking of the 'real' functions, variables,
> values, whatsoever.

What are you checking, though?  What kind of error condition are you trying to catch?  Conditions that I can think of are

 *  File is missing -- but in that case Bash will tell you when sourcing it
    and/or you can check for it explicitly (or make it impossible to "find" such
    path in the first place).

 *  File fails with syntax error -- as above: it's trivial to check for this
    explicitly (`bash -n`).  Alternatively you could require lib.sh to exit with
    zero, like Perl does.

 *  User messed up prefix -- nice, but seeing they did not mess up prefix for
    this particular function only tells you that, ie. they did not mess up
    prefix for fooLibraryLoaded.

 *  User called "naked" return earlier -- OK, but again, as a check, it would
    only work if they did it before defining *LibraryLoaded.

 *  Bash itself somehow fails -- but then you don't have rlImport problem but
    "this is haunted" problem; there's no reason to expect rlImport() would even
    exist at that point.

It's a pretty useless check, is what I'm saying.  Superficial at best.