Test enhancements

[dcurrie]

Add a means for (picrin test) cases to expect an error and "pass" if the error is received, "fail" if not.

Add a test case t/substring-index.scm to test this feature along with yesterday's change to string-copy to check indices.

My git-fu ls lacking... this two file scm patch picked up the string-copy c changes from yesterday.

[nyuichi]

@dcurrie

Hm, :error: looks somehow wired to me. How about using a syntactic keyword like

(test error "error is thrown"
  expr ...)

The code of the test macro that implements this feature should look like

(define-syntax test
  (syntax-rules (error)
    ((test error msg expr ...)
     # expansion
     ...)
    ((test msg expr ...)
     # normal case
     ...)
    ...)))

I don't think the test macro has to handle cases in which general objects that are not of error-object? are thrown. For such the cases we can add another API (test-raise or something).

[dcurrie]

I considered a syntactic keyword, but it makes the logic a lot more complicated since there are now three cases instead of one, and they all need the same code, essentially. The :error: symbol is what we call a keyword in Common Lisp, a symbol that evaluates to itself. It's exported from (picrin test) so it doesn't pollute the normal environment. I don't care what it's called, though I'm accustomed to keywords starting with :. The ending : is a nice match for the "error: ..." syntax used by the repl's (display "error: ")

My initial motivation for this change is precisely because the general case is not checking for errors. Even when errors are not expected, if they occur they should not stop the test from proceeding; they should just be reported as a failure. An example is the test t/issue/282.scm -- without my (picrin test) patch it does not complete because the third test against "-1.0" has an invalid index. We can also use this mechanism to fix other test files, like t/106.scm that kill the test script on OS X. So, I think test-raise is the wrong approach since you never know where errors will occur.

[nyuichi]

@dcurrie

it makes the logic a lot more complicated since there are now three cases instead of one, and they all need the same code, essentially

Definitely. Syntax-rules is very poor at such abstraction. Wish we had a better alternative to syntax-rules.

I'm accustomed to keywords starting with :

So do I. :) But :-prefixed keyword is not as common as other lisps in scheme.

My initial motivation for this change is precisely because the general case is not checking for errors.

Two issues seems mixed up. Handling uncaught errors is a different problem to expect-an-error tester and we have to consider them separately. When testing an expression with (test "foo" expr) and expr contains a bug that produces an unintentional exception, we suppose test to say something like we had an unexpected error while running 'expr' but it doesn't for now (bug 1). And as of now we don't have a mean to test expressions that must fail (expect an error is thrown) (bug 2). So, for bug1, I think test macro should do all handlings including warnings emission. Telling by returning special values that expression is done with an error sounds not so bad an idea but basically test macro should do its task by doing writing messages to console. We can fix this bug just by modifying your patch some lines. The second bug is a feature request rather than a bug. The API is still remained to be discussed and I will make a proposal patch.

Thoughts?

[dcurrie]

Two issues seems mixed up.

Well, yes and no. I understand your perspective. My perspective is that a scheme form can return a value or it can generate an error, and that the test framework is designed to test if the form does the "right" thing or the "wrong" thing. The wrong thing can be a bad result, or an unexpected error. It counts as a "fail" and displays that way. The right thing can be a good result, or an expected error. It counts as a "pass" and displays that way.

basically test macro should do its task by doing writing messages to console

I believe my patch does "the right thing" in this case, e.g.,

> (test "oo" (substring "foo" 1 4))
case 0 FAIL: (substring "foo" 1 4)
   expected "oo" but got (:error: "string-copy: invalid index")

second bug is a feature request rather than a bug

Yes, but my patch does that, too, for free!

[nyuichi]

@dcurrie

expected "oo" but got (:error: "string-copy: invalid index")

I got your point. So I was wrong about the expansion result.

Yes, but my patch does that, too, for free!

Right. Your patch solves the problems at a time. But I'm not sure if it is "the right way". Hmm, let me think for a while.