Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test cl-gserver/tests/actor-test.lisp not actually good #99

Open
Yehouda opened this issue Dec 11, 2024 · 1 comment
Open

Test cl-gserver/tests/actor-test.lisp not actually good #99

Yehouda opened this issue Dec 11, 2024 · 1 comment

Comments

@Yehouda
Copy link

Yehouda commented Dec 11, 2024

(test actor-of--limit-queue-size

The test assumes that tell will succed only once, but if the actor starts up it may take one of the events before the loop calling tell ends, in which case two calls will succeed.
In our version I changed it to:

  1. sleep before starting the loop, so always two calls are succeessful.
  2. check for 2 successes and 8 failures, rather than 1 and 9.
  3. also sleep before the shutdown.

BTW: the fact that shutdown error if any of the actors has a full queue looks to me like a bug, or at least a mis-feature. There should be a way to shutdown the system independently of what the actors do.

(test actor-of--limit-queue-size
         "Tests that actor's queue size can be limited."
         (let ((sys (asys:make-actor-system)))
           (unwind-protect
               (let* ((actor (actor-of sys
                                       :receive (lambda (msg)
                                                  (declare (ignore msg))
                                                  (sleep 0.05))
                                       :queue-size 1))
                      (tells
                       (progn (sleep 0.01) ; make sure the actor started
                         (loop :repeat 10

                               :collect (ignore-errors
                                          (tell actor "run"))))))
                 (format t "tells: ~a~%" tells)
                 (is (= 2 (length (filter (lambda (x) (if x x)) tells))))
                 (is (= 8 (length (filter #'null tells)))))
             (sleep 0.1)
             (ac:shutdown sys))))
@mdbergmann
Copy link
Owner

Yeah, you are right. This test is a bit flaky.
Dealing with timings is always problematic.
actor-of returns a fully initialized actor ready to process messages, so I'm not sure the initial sleep is necessary.
The 2/8 partitioning is a good idea. Could be a bit more reliable.

The problem with shutdown are the threads that are trying to pop the mailbox queue. If there is no message in the queue the pop operation the thread executes will block. So in order to leave a clean system after ac:shutdown one has to make sure the threads are properly stopped/cancelled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants