New reference implementation taylanbayirli@xxxxxx (29 Aug 2015 17:38 UTC)
Re: New reference implementation Per Bothner (29 Aug 2015 20:23 UTC)
Re: New reference implementation taylanbayirli@xxxxxx (29 Aug 2015 21:38 UTC)
Re: New reference implementation Per Bothner (30 Aug 2015 09:20 UTC)
Re: New reference implementation taylanbayirli@xxxxxx (30 Aug 2015 10:45 UTC)
Re: New reference implementation taylanbayirli@xxxxxx (31 Aug 2015 21:22 UTC)
Re: New reference implementation Per Bothner (31 Aug 2015 22:11 UTC)
Re: New reference implementation taylanbayirli@xxxxxx (01 Sep 2015 08:44 UTC)
Re: New reference implementation Per Bothner (01 Sep 2015 10:44 UTC)
Re: New reference implementation John Cowan (30 Aug 2015 01:24 UTC)
Re: New reference implementation Arthur A. Gleckler (30 Aug 2015 04:35 UTC)
Re: New reference implementation John Cowan (30 Aug 2015 17:10 UTC)
Re: New reference implementation Per Bothner (30 Aug 2015 05:06 UTC)
Re: New reference implementation taylanbayirli@xxxxxx (30 Aug 2015 08:06 UTC)
Re: New reference implementation Per Bothner (30 Aug 2015 08:25 UTC)
Re: New reference implementation taylanbayirli@xxxxxx (30 Aug 2015 08:49 UTC)
Re: New reference implementation Per Bothner (30 Aug 2015 09:33 UTC)
Re: New reference implementation taylanbayirli@xxxxxx (30 Aug 2015 12:35 UTC)
Re: New reference implementation taylanbayirli@xxxxxx (22 Sep 2015 21:27 UTC)
Re: New reference implementation Per Bothner (24 Sep 2015 00:25 UTC)
Re: New reference implementation taylanbayirli@xxxxxx (24 Sep 2015 08:26 UTC)
Re: New reference implementation taylanbayirli@xxxxxx (26 Sep 2015 11:49 UTC)
Re: New reference implementation Per Bothner (28 Sep 2015 17:47 UTC)
Re: New reference implementation taylanbayirli@xxxxxx (28 Sep 2015 19:54 UTC)
Re: New reference implementation Per Bothner (02 Oct 2015 06:07 UTC)
Re: New reference implementation Per Bothner (02 Oct 2015 06:36 UTC)
Re: New reference implementation taylanbayirli@xxxxxx (02 Oct 2015 09:39 UTC)
Re: New reference implementation Per Bothner (06 Oct 2015 21:13 UTC)
Re: New reference implementation taylanbayirli@xxxxxx (07 Oct 2015 09:17 UTC)

Re: New reference implementation taylanbayirli@xxxxxx 28 Sep 2015 19:55 UTC

Per Bothner <xxxxxx@bothner.com> writes:

> Some concerns:
>
> * I still don't see any line number information in the output.

It seems to work for some forms but not all.  I might need some help on
this, here's the story:

I really wish not to litter the main body of code with big cond-expand
blocks that conditionally bind test-assert, test-eqv, etc. to
syntax-case macros with source location gathering when supported, and
syntax-rules macros otherwise.  So instead I use the source-info.sld
sub-library which defines a handy 'source-info' macro which will just
return #f behind the scenes when syntax-case is not supported, and
otherwise return the location at which it was used.

In Guile, when the body of a syntax-rules macro calls (source-info), I
end up with the source info of the call to the syntax-rules macro,
i.e. the place into which '(source-info)' expanded.  So the following
works fine:

    (define-syntax test-assert
      (syntax-rules ()
        ((_ ...)
         (%test-assert (source-info) ...))))

(ellipsis used meta-syntactically).

But in Kawa, it yields the real lexical position of the '(source-info)'
call, which of course is not what we want in this case; all source info
ends up pointing into the SRFI-64 implementation.

I really don't know which syntax-case behavior is more desired in the
general case.  The one of Guile eases our job in this case, but maybe
it's undesired in general.  If you say it's desired and can implement
it, that would fix things, but I assume it would be undesired and/or
nontrivial to implement, so:

To work around that, I now effectively do something like the following
for Kawa (it in turn fails on Guile, so I had to do a bit of cond-expand
dancing, but still very well encapsulated):

    (define-syntax source-info
      (lambda (stx)
        (syntax-case stx
          ((_ x)
           (get-source-info-for (syntax x))))))

    (define-syntax test-assert
      (syntax-rules ()
        ((_ foo ...)
         (%test-assert (source-info foo) ...))))

(ellipsis still just used meta-syntactically)

i.e. 'source-info' is given a specific form whose source info it should
yield, which I find pretty logical when I think about it.

In the real code, the form passed to 'source-info' is the "expression"
subform of test-assert.  That means even the following should
theoretically work well:

    (define-syntax test-foo
      (syntax-rules ()
        ((_ expr blah blub)
         (begin
           blah blub
           (test-assert name expr)))))

    (test-foo expr1 x y)
    (test-foo expr2 a b)

That would ultimately report the source location of the two uses of
test-foo, because those are where the individual 'expr' instances come
from (unless test-foo is in turn wrapped in yet another macro).

If you say you can make this work, that would be pretty amazing and
solve the problem in the current implementation.  I suspect the source
info of the expr gets lost when it's generated by a macro or so.

But if that's also nontrivial to implement, then I have a last-resort
solution in mind that involves using a macro which expands to macro
definitions, but conditionally either to a syntax-case macro definition
or to a syntax-rules macro definition.  That way big cond-expand blocks
are still avoided in the main body of code.

Tell me which way I should proceed.

> * I think I'd much rather see (current reference output):
>
> ./r7rs-tests.scm:494: FAIL
>
> than:
>
> [FAIL] R7RS/4.3 Macros/val
>
> though a combination might be even better:
>
> ./r7rs-tests.scm:494: FAIL (R7RS/4.3 Macros/val)
>
> Note having the line number first, and with this syntax, is both
> traditional and handled better by tools such as Emacs.
>
> If line numbers aren't available, the following works:
>
> FAIL (R7RS/4.3 Macros/val)
>
> If you really prefer the square brackets, the following are acceptable:
>
> ./r7rs-tests.scm:494: [FAIL] R7RS/4.3 Macros/val
> [FAIL] R7RS/4.3 Macros/val

The source location is already reported as follows when available:

Source:
./lib-test.scm:623

That is an extra couple lines after the [FAIL] line.  I believe that
should satisfy Emacs and other systems?

I suppose you missed all the extra lines in the output (although in the
case of source location it's really missing sometimes) ...

> * The log file should report both expected and actual result values, at
> least in the case of failure.

Like the source info, these are printed in extra lines (both in stdout
and the log file).

> * The current reference implementation also reports in the log file
> the source-form, but that is probably not as important or useful

Likewise.

> * The standard output should only report unexpected failures (and passes),
> so to make runs "more quiet".  That's the reason for the log file: to be able
> to look at the details *when you want/need to*, not all the time.

I just imitated the behavior I'm used to seeing from running 'make
check' in most GNU programs.  The rationale would probably be that the
silence could become irritating when the test suite takes a long time
and when no tests fail, which should be the normal case. ;-)

In any case 'make check' (or whatever program is used to run the test
suite) should probably report failure through a non-zero exit code.
After that the user can search for "FAIL", and the precise output format
becomes mostly moot so long as all useful information is there in some
way (though being Emacs-parsable is pretty neat).

> *I'd also skip (at least in standard output) the summary lines with count 0,
> and the group begin/end messages.

Hmm, I like to see a consistent format (same lines in same order every
time, only counts changing), and the code is marginally simpler this
way.  I'd like to keep the current format unless there's a good reason
not to.

By the way: testsuite/arr-test.scm lines 347 and 353 contain excess
arguments to test-equal.  It thus tries to use an integer for the test
name.  (This errors in my implementation; I guess the original somehow
allows it although I don't think it should.)

Taylan