<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/xhtml; charset=utf-8">
</head>
<body>
<div style="font-family:sans-serif"><div style="white-space:normal">
<p dir="auto">On 30 Sep 2019, at 6:23, Anton Vodonosov wrote:</p>
</div>
<div style="white-space:normal"><blockquote style="border-left:2px solid #777; color:#777; margin:0 0 5px; padding-left:5px"><p dir="auto">Hi.<br>
<br>
Thank you, everyone, for addressing this topic, having a unified representation<br>
of test results would be very useful.<br>
<br>
A couple of thoughts:<br>
<br>
<br>
- success should also be signaled, so we can distinguish a version where<br>
this new protocol is not implemented from the version where tests pass</p>
</blockquote></div>
<div style="white-space:normal">
<p dir="auto">This requires a protocol where ASDF can "know" when the test op is done, so that it can distinguish "has succeeded" from "has not failed yet." It's possible that this could be managed by the test op on the system as a whole, but the design must consider the possibility that the test-op may be implemented so that:</p>
<ol>
<li value="1">Multiple calls are made into the test library (possibly even multiple test libraries are used)</li>
<li value="2">TEST-OP may involve invoking test operation on systems depended on (e.g., where a system has subsystems).</li>
</ol>
</div>
<div style="white-space:normal"><blockquote style="border-left:2px solid #777; color:#777; margin:0 0 5px; padding-left:5px"><p dir="auto">- the minimal requirement is a success / failure designator, the failed<br>
test names can be optional</p>
</blockquote></div>
<div style="white-space:normal">
<ul>
<li>Additional requirement: the condition should support accessing both a long and a short form report. In degenerate implementations, these can, of course, be identical.</li>
</ul>
</div>
<div style="white-space:normal"><blockquote style="border-left:2px solid #777; color:#777; margin:0 0 5px; padding-left:5px"><p dir="auto">- For a caller of asdf:test-op it would be more convenient to have a single<br>
signal. Ideally, it should be just a return value of the asdf:operate function,<br>
as I understand we only consider the possibility of test result being signaled<br>
multiple times during test-op because we hope to make it work for everyone<br>
without library authors explicitly modify their code, but adding this new<br>
functionality to test frameworks. A good goal, although I can imaging<br>
some corner cases. Still, even if we expect test results being signalled<br>
multiple times during a test-op, it would be good to provide a wrapper<br>
which aggregates them into a single return value.<br>
<br>
(common-test-results:collect (asdf:test-system "my-system"))</p>
</blockquote></div>
<div style="white-space:normal">
<p dir="auto">This would require that the test library provide an extensible protocol for fusing together multiple test results. And note that the above suggestion will not work, because ASDF does not ever <em>return</em> a value from operating. This has to do with the way ASDF creates a plan and then executes it. The plan doesn't support a notion of "return value," so the only way to get information out of ASDF is through conditions.</p>
<p dir="auto">One could, possibly, make <code style="background-color:#F7F7F7; border-radius:3px; margin:0; padding:0 0.4em" bgcolor="#F7F7F7">asdf:test-system</code> have a return value by making it handle conditions, but that would break the current equivalence that <code style="background-color:#F7F7F7; border-radius:3px; margin:0; padding:0 0.4em" bgcolor="#F7F7F7">asdf:test-system</code> is just a shorthand for calling <code style="background-color:#F7F7F7; border-radius:3px; margin:0; padding:0 0.4em" bgcolor="#F7F7F7">OPERATE</code>.</p>
</div>
<div style="white-space:normal"><blockquote style="border-left:2px solid #777; color:#777; margin:0 0 5px; padding-left:5px"><p dir="auto">- as others mention, to me it also occurred this new functionality<br>
should not necessarily be declared inside of ASDF, it could be<br>
some separate library, say common-test-result. I'm not 100% sure<br>
about this, but currently, lean more towards separate lib, at least<br>
for the beginning. ASDF test-op docs could just referer to it.</p>
</blockquote></div>
<div style="white-space:normal">
<p dir="auto">I agree -- I think <code style="background-color:#F7F7F7; border-radius:3px; margin:0; padding:0 0.4em" bgcolor="#F7F7F7">TRIVIAL-TEST-INTERFACE</code> might be a better first step. I suppose the alternative rationale is that a test interface that was <em>not</em> intended for incorporation into ASDF would be able to just <em>return</em> things, instead of <em>signaling</em> them.</p>
</div>
<div style="white-space:normal"><blockquote style="border-left:2px solid #777; color:#777; margin:0 0 5px; padding-left:5px"><p dir="auto">- If delivering test results thourhg a condition, test failure should not<br>
be an error or warning, in my opinion. Test error is an anticipated<br>
possible outcome. An error during tests an abnormal situation -<br>
no access to needed files, memory exhausted, null pointers, etc.</p>
</blockquote></div>
<div style="white-space:normal">
<p dir="auto">That is true, but it's also true that it would require special condition-handling to fit test results into continuous integration -- programmers would no longer be able to just use <code style="background-color:#F7F7F7; border-radius:3px; margin:0; padding:0 0.4em" bgcolor="#F7F7F7">quit-on-error</code>, which is a very handy way to turn a lisp test into something that works in Jenkins or, for that matter, any infrastructure based on shell scripting.</p>
<p dir="auto">I'd rather have to write code to handle errors when I <em>don't</em> want them, than have test failure not be an error.</p>
<p dir="auto">If I'm running interactively, it's not a bother to deal with this as an error condition -- I can easily get out of the debugger. But writing a lot of code to catch <code style="background-color:#F7F7F7; border-radius:3px; margin:0; padding:0 0.4em" bgcolor="#F7F7F7">TEST-FAILURE</code> conditions and translate them into exit with non-zero status would be a pain.</p>
<p dir="auto">A solution might be to have a top-level handler that can turn these conditions into errors, or not, as appropriate. But unfortunately, it's not at all easy to pass information from top-level calls to ASDF operations into the way those operations are executed, since <code style="background-color:#F7F7F7; border-radius:3px; margin:0; padding:0 0.4em" bgcolor="#F7F7F7">OPERATION</code> objects no longer carry attributes (Faré removed them because attribute propagation never worked correctly, but sometimes I still regret this).</p>
</div>
<div style="white-space:normal"><blockquote style="border-left:2px solid #777; color:#777; margin:0 0 5px; padding-left:5px"><p dir="auto">- slot for the failing asdf system could probably be avoided,<br>
the list failed test names could be enough, if the names are "fully qualified"<br>
i.e. include package or system name.</p>
</blockquote></div>
<div style="white-space:normal">
<p dir="auto">I don't think we can make any assumptions about the above -- there's no rule about how a programmer can assign test names in a library like FiveAM to packages. Similarly, when writing tests, the programmer does not generally put in the tests information about the containing system -- indeed, doing so would be a violation of standard notions of abstraction (containers know about the contained; contained things don't have to know about their containers).</p>
<p dir="auto">Some kind of dynamic binding could allow these condition objects to automatically collect information about the system under test.</p>
<p dir="auto">This is such a complex issue that we should either have a pretty substantial design before we put it into ASDF, or we should kick this out of ASDF into another library.</p>
<p dir="auto">I'm not willing to incorporate into ASDF something that would incur substantial maintenance debt, without very high confidence that the design is solid. This will have tentacles everywhere.</p>
<p dir="auto">I would note also that getting a new library into Quicklisp for this is going to be a lot easier than getting a new ASDF into Quicklisp: Xach has for years refused to update the ASDF version in Quicklisp, and I don't see any reason to believe this will change.</p>
</div>
<div style="white-space:normal"><blockquote style="border-left:2px solid #777; color:#777; margin:0 0 5px; padding-left:5px"><p dir="auto">27.09.2019, 10:20, "Vladimir Sedach" <vas@oneofus.la>:</p>
<blockquote style="border-left:2px solid #777; color:#999; margin:0 0 5px; padding-left:5px; border-left-color:#999"><p dir="auto">Thank you for the specific suggestions Mark.<br>
<br>
Mark Evenson <evenson@panix.com> writes:<br>
</p>
<blockquote style="border-left:2px solid #777; color:#BBB; margin:0 0 5px; padding-left:5px; border-left-color:#BBB"><p dir="auto"> 1. Have a slot in your base condition class TEST-OP-TEST-FAILURE in<br>
which one can record the ASDF component which caused the failure.<br>
It is probably possible to dig this information out of the stack,<br>
but that will be messy. This would also allow for distinguishing<br>
when multiple TEST-OP-TEST-FAILURES are signaled from a single<br>
ASDF:TEST-OP invocation, as will be the case when one “chains” test<br>
invocation over many ASDF systems.</p>
</blockquote><p dir="auto">This is really easy to do with a special variable in an initarg, but<br>
are there any systems that you know of that do this? I would<br>
definitely like to test with them, because I thought that nested<br>
TEST-OP was not supposed to work. From the "More Elaborate Testing"<br>
section of the best practices document¹:<br>
<br>
"You MUST NOT call asdf:operate or any of its derivatives, such as<br>
asdf:load-system or asdf:test-system from within a perform method."<br>
<br>
Unfortunately it looks like that is what ROVE:RUN-SYSTEM-TESTS does<br>
exactly that.<br>
</p>
<blockquote style="border-left:2px solid #777; color:#BBB; margin:0 0 5px; padding-left:5px; border-left-color:#BBB"><p dir="auto"> 2. Provide an implementation of the subclass of<br>
TEST-OP-TEST-FAILURE that contains the basic structure of a<br>
reporter class for the information that should be present in all<br>
test frameworks, namely the total number of tests run, the number<br>
of failed tests, the identities of the failed tests, and a slot for<br>
a human readable error message, along with a reporter function that<br>
displays this information. Having an implementation class to work<br>
from would make it easier for test frameworks to adapt.</p>
</blockquote><p dir="auto">I tried to avoid enforcing required slots, but as both<br>
asdf-test-harness and cl-test-grid want a list of failed tests, that<br>
is a strong case to make the above slots required in<br>
TEST-OP-TEST-FAILURE itself.<br>
<br>
cl-test-grid wants a list of test names as strings (it wants them<br>
down-cased, but that is a detail that can be left to cl-test-grid). A<br>
list of strings is a requirement that any test library should be able<br>
to satisfy (worst case, it could be a list of random names), and<br>
looks to me specific enough for most test harness use cases.<br>
<br>
The length of the list of failed test names gives the count of failed<br>
tests.<br>
<br>
It seems to me like having a slot for an error message is redundant<br>
with the reporter function, given that I think it should be up to the<br>
test library to define the reporter function, and not for<br>
TEST-OP-TEST-FAILURES to dictate how it is printed. That way, if a<br>
test library has a flag to print results in machine readable format,<br>
the flag will work without any changes if the overridden reporter<br>
function re-uses the library's output facilities, and as long as the<br>
test harness PRINCs the condition, the test harness does not need to<br>
do anything either.<br>
</p>
<blockquote style="border-left:2px solid #777; color:#BBB; margin:0 0 5px; padding-left:5px; border-left-color:#BBB"><p dir="auto"> 3. Go ahead and define the subclass of this condition when no tests<br>
have been run.</p>
</blockquote><p dir="auto">I thought about doing this, but with the above slots, there is no<br>
need to - the test library can signal TEST-OP-TEST-FAILURE with a 0<br>
count of total number of tests run.<br>
</p>
<blockquote style="border-left:2px solid #777; color:#BBB; margin:0 0 5px; padding-left:5px; border-left-color:#BBB"><p dir="auto"> 4. As for adoption by test framework your strategy, we will have<br>
the problem that a given test framework won’t want to adopt the<br>
conditions because it isn’t in the version of ASDF they are using,<br>
or can easily get a hold of. To solve this, we might somehow define<br>
the code within the ASDF source tree so that one can make a<br>
standalone ASDF system (“ASDF-TEST-CONDITIONS” or some such) that<br>
one may include separately from actually upgrading ASDF.</p>
</blockquote><p dir="auto">That is something that Robert brought up on the merge request<br>
discussion.² It looks like this can be handled with the #+ #- feature<br>
macros or #. read macro to provide CL:WARNING as a fallback<br>
super-class. I am open to any ideas.<br>
<br>
I went ahead and added the slots to ASDF², the FiveAM³, and the rove⁴<br>
implementations. Up next, I am going to work on adding support to<br>
cl-test-grid for a library that uses rove, which cl-test-grid does<br>
not support yet.<br>
<br>
¹ <a href="https://github.com/fare/asdf/blob/master/doc/best_practices.md" style="color:#999">https://github.com/fare/asdf/blob/master/doc/best_practices.md</a><br>
² <a href="https://gitlab.common-lisp.net/asdf/asdf/merge_requests/124" style="color:#999">https://gitlab.common-lisp.net/asdf/asdf/merge_requests/124</a><br>
³ <a href="https://github.com/sionescu/fiveam/pull/58" style="color:#999">https://github.com/sionescu/fiveam/pull/58</a><br>
⁴ <a href="https://github.com/fukamachi/rove/pull/29" style="color:#999">https://github.com/fukamachi/rove/pull/29</a><br>
<br>
--<br>
Vladimir Sedach<br>
Software engineering services in Los Angeles <a href="https://oneofus.la" style="color:#999">https://oneofus.la</a></p>
</blockquote></blockquote></div>
<div style="white-space:normal">
</div>
</div>
</body>
</html>