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

Events bleed between tests using run-test-async #13

Open
jasich opened this issue Aug 22, 2017 · 10 comments
Open

Events bleed between tests using run-test-async #13

jasich opened this issue Aug 22, 2017 · 10 comments

Comments

@jasich
Copy link

jasich commented Aug 22, 2017

I have a test where I want to check for one specific event using rf-test/wait-for. This event cascades some dispatches of other events, which seem to bleed into other async tests if I don't explicitly wait for them to occur. Ideally if the test ends re-frame-test would kill all pending events. My current workaround is to add a wait-for at the end of the test for the last event in the event chain so that pending event's don't infect the other tests.

@danielneal
Copy link

danielneal commented Aug 23, 2017

I'm seeing this same problem. Specifically for me, I had some irrelevant dispatch-laters which were starting to bleed in to subsequent tests.

I'm working around it with the following horror

(def *timers (atom #{}))

(defn monkey-patch-reframe-timers! []
  (set! re-frame.interop/set-timeout!
        (fn [f ms]
          (let [id (js/setTimeout f ms)]
            (swap! *timers conj id)
            id))))

(defn clear-timers! []
  (doseq [t @*timers]
    (js/clearTimeout t))
  (reset! *timers #{}))

And calling clear-timers! as part of test setup, and monkey-patch-reframe-timers! from the test runner.

An official solution would be very, very welcome!!

@mike-thompson-day8
Copy link
Contributor

mike-thompson-day8 commented Oct 30, 2017

@jasich checking: is the solution simply that run-test-async clears all events from re-frame's queue when it exits?

@danielneal yes, you'll have to stub out the effect handler for :dispatch-later so it doesn't actually dispatch later. run-test-async will restore all original handlers when it exits.

(reg-event-fx  :dispatch-later  (fn [_] ))

@danielneal
Copy link

@mike-thompson-day8 definitely something like that.
Would clearing re-frame's queue clear up the dispatch-laters though?
https://github.com/Day8/re-frame/blob/cac2c7a02e6680f4bf43d9b3d3cf72a2a39a3209/src/re_frame/fx.cljc#L88-L94 Looks like it boils down tojs/setTimeout, I couldn't make out if there is a reframe layer queue of these that can be cleared, or if those timeouts just kinda hang around.

@mike-thompson-day8
Copy link
Contributor

mike-thompson-day8 commented Oct 30, 2017

@danielneal Sorry, I edited/added-to my last answer at about the same time you responded. So you might not have seen everything I ended up putting in there (my bad, I know). Can you have another read of the above please.

I think I see two different issues here (solved in different ways):

  1. Clearing the re-frame event queue (because there are unneeded events ALREADY on the queue)
  2. Stopping :dispatch-later events from ever getting added LATER to the queue.

@danielneal
Copy link

danielneal commented Oct 30, 2017

@mike-thompson-day8 Yes that sounds right! A stub of dispatch later (and any user defined fx that might leak via timeouts) would handle the slow bleed, and clearing the event queue should handle the fast bleed of events that are already queued up (has usually been just ~one pending event in my experience).

@danielneal
Copy link

danielneal commented Oct 30, 2017

@mike-thompson-day8 I'll go and do (2) right now - what should I do for (1) to call to clear the reframe event queue? Is that a change to re-frame-test or something I should be doing from my test code.

@mike-thompson-day8
Copy link
Contributor

mike-thompson-day8 commented Oct 30, 2017

@danielneal re-frame doesn't currently supply an API for purging events. It probably should. We can add that. day8/re-frame#425

Once that's in place, then the macro run-test-async can be changed to call this API

@mike-thompson-day8
Copy link
Contributor

mike-thompson-day8 commented Oct 30, 2017

As a first step, I've hastily added a new API function re-frame.core/purge-event-queue. This new code in the re-frame repo is untested at this point. That will be handled tomorrow. But there is enough there for someone to think about creating a PR for the necessary, further changes to run-test-async

@jasich
Copy link
Author

jasich commented Oct 31, 2017

@mike-thompson-day8, I believe that clearing the queue at the end of run-test-async should fix the issue I was having. I wonder if you'd want to communicate that the queue is clearing {x} number of events at the end of an async test though. It might be good information when you're trying to debug a test issue. Also thanks for your work on re-frame & re-frame-test!

@spacegangster
Copy link

spacegangster commented Nov 4, 2019

Hello @mike-thompson-day8 and the maintainers. Firstly, thank you for your awesome work!
If I could send a couple of bucks a month to fund further work – I would gladly do that :)

Official fix is still relevant. My run-test-async don't finish, until I exclude tests for the code that uses dispatch later.

I'm using slightly modified code from @danielneal, and tests seem to work. (They work even with the original piece).

(def *timers (atom #{}))

(defn monkey-patch-reframe-timers!
  "call me from your test runner"
  []
  (rf/reg-event-fx  :dispatch-later  (fn [_]))
  (set! re-frame.interop/set-timeout!
        (fn [f ms]
          (let [id (js/setTimeout f ms)]
            (swap! *timers conj id)
            id))))

(defn clear-timers!
  "call me in the beginning of your deftests"
  []
  (rf/purge-event-queue)
  (doseq [t @*timers]
    (js/clearTimeout t))
  (reset! *timers #{}))

I've tried just (purge-event-queue) in the beginning of my deftest's, it didn't help.
If you need more input we can schedule a call.
If this workaround is good enough – I can incorporate it, run tests and do a PR.

Thanks again for the tools!

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

4 participants