Shinmera / parachute

An extensible and cross-compatible testing framework.
https://shinmera.github.io/parachute
zlib License
94 stars 9 forks source link

PARACHUTE::PRINT-ONELINE does not handle circular structures #38

Open phoe opened 2 years ago

phoe commented 2 years ago
(define-test foo
  (is equal 42 (alexandria:make-circular-list 1)))

VALUE-SEMANTICS/TEST> (parachute:test 'foo)

        ? VALUE-SEMANTICS/TEST::FOO
  0.000 ✘   (is equal 42 (make-circular-list 1))
  0.008 ✘ VALUE-SEMANTICS/TEST::FOO

;; Summary:
Passed:     0
Failed:     1
Skipped:    0

;; Failures:
   1/   1 tests failed in VALUE-SEMANTICS/TEST::FOO

And it just keeps on going, trying to print nil into its string input stream until it runs out of heap. Should it bind *print-circle* perhaps?

phoe commented 2 years ago

Actually it seems that (parachute:test 'foo :report 'parachute:interactive) hangs as well.

Shinmera commented 2 years ago

Just set *print-circle* to T

phoe commented 2 years ago

Tried that, didn't work.

CL-USER> (parachute:define-test foo (parachute:is equal 42 (alexandria:make-circular-list 1)))
FOO
CL-USER> (setf *print-circle* T)
T
CL-USER> (parachute:test 'foo)
        ? COMMON-LISP-USER::FOO
  0.004 ✘   (is equal 42 (make-circular-list 1))
  0.016 ✘ COMMON-LISP-USER::FOO

;; Summary:
Passed:     0
Failed:     1
Skipped:    0

;; Failures:
   1/   1 tests failed in COMMON-LISP-USER::FOO

And it hangs.

Shinmera commented 2 years ago

Hmm, odd.

phoe commented 2 years ago

I know, right. I suspect a with-standard-io-syntax or something similar somewhere, but that's just a wild guess.

phoe commented 2 years ago

It isn't. *print-circle* is true in the dynamic environment of the test - see the screenshot. Zrzut ekranu z 2022-01-06 21-51-32

phoe commented 2 years ago

Found it!

https://github.com/Shinmera/parachute/blob/86563473dc23fb1277d35a3ad2c911a6c8e5b0da/toolkit.lisp#L69-L78

This loop form is going to loop forever because (loop for (car . cdr) on '#1=(nil . #1#)) won't terminate.

Shinmera commented 2 years ago

Rrright, since we need to customise each element printed.

phoe commented 2 years ago

More fireworks:

CL-USER> (parachute:define-test foo (parachute:is equal 42 #1=#(1 2 3 #1#)))
FOO

CL-USER> (parachute:test 'foo)
        ? COMMON-LISP-USER::FOO
  0.000 ✘   Control stack guard page temporarily disabled: proceed with caution
phoe commented 2 years ago

If you reimplement the Lisp printer inside print-oneline, you'll need to reimplement *print-circle* handling as well.

Is there a particular reason you're not going for something like (let ((*print-right-margin* most-positive-fixnum)) ...) and using the standard Lisp printer? I see one reason so far, printing NIL as (). But that could be worked around via a pprint dispatch I think.

Shinmera commented 2 years ago

Generally I need to bypass lisp's really annoying way of breaking shit onto multiple lines.

phoe commented 2 years ago

Hmmmm. I'd set the print-width to maximum, let Lisp print everything normally. Should work if we go with format ~A/princ.

Any edge cases that this doesn't cover? Because then we could substitute all whitespace characters with spaces, then replace all runs of 2+ spaces with a single space. Slimy, yet satisfying.

Shinmera commented 2 years ago

In my experience fiddling with print-width or whatever doesn't help at all.

phoe commented 2 years ago

Hmmmmmmm.

I'd go for the brutal&blunt second solutionworkaround I described then. I see no point in reinventing the Lisp printer unless we want to reinvent circularity handling too.

Shinmera commented 2 years ago

I don't like string replacement attempts because it'll inadvertently muck with user data, unless we reimplement the lisp reader :)

phoe commented 2 years ago

Sure, that sounds good as well, especially in case of cases like (is string= " " string).

Regardless of the way this goes, hanging and/or blowing the heap and/or the stack when circular structures are present in the test report is something that I'd consider fixworthy. Thankfully the positive cases are not affected, only negative ones.

Shinmera commented 2 years ago

I'd sooner implement a print-circle fixup than do other things is what I'm saying.

phoe commented 2 years ago

OK, understood - then the way forward is standard:

tfeb commented 1 year ago

I have now run into this bug as well. Since I am working on tests for some code which is entirely about dealing with circular and sharing structure this means that every time a test tries to print anything the Lisp explodes in some exciting way, either chewing through all the memory of the machine or killing the stack. This is ... less than useful behaviour for a test framework.

Dealing with circularity properly is obviously hairy. tfeb/parachute/print-circle contains something which at least prevents it from exploding: it just checks whether *print-circle* is true at the point it's about to print a cons and, if it is, binds *print-lines* to 1 and calls princ, which knows how to avoid explosion.

Absent a proper solution (I don't have the enthusiasm to try and reimplement half of the Lisp printer) this does at least stop it exploding and make it usable. I'd suggest something like this as an interim fix, at least.

Shinmera commented 1 year ago

@tfeb I'd be happy to merge that, can you PR it?

tfeb commented 1 year ago

@Shinmera done. I think it's no more than an interim fix!

Shinmera commented 1 year ago

I understand. Thanks!