day8 / re-frame-test

Cross platform (cljs and clj) utilities for testing re-frame applications
MIT License
109 stars 12 forks source link

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

Open jasich opened 7 years ago

jasich commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

@danielneal re-frame doesn't currently supply an API for purging events. It probably should. We can add that. https://github.com/Day8/re-frame/issues/425

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

mike-thompson-day8 commented 7 years ago

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 commented 7 years ago

@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 commented 5 years ago

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!