Timeout for `wait-for`

[jeaye]

Assuming a test fails and there's neither a success event sent nor a failure event, it looks like the current behavior is to wait forever. How do you feel about having a configurable timeout?

I imagine this would happen not due to a network timeout, which would ultimately return an error, but perhaps because some code or data was missing which should be there, so no event is ever sent.

[jeaye]

Any update on this?

[samroberton]

For tests running on the JVM, wait-for will in fact timeout after rf-test/*test-timeout*.

For tests running in a JS environment, wait-for doesn't actually wait -- it returns immediately, having installed a callback handler that will be run if and when the event(s) you're waiting for are dispatched.

Instead of wait-for waiting, what actually happens is that your test reaches the end of its body, but since it's an async JS test that hasn't yet signalled completion to the test framework, the test framework considers that the test is still executing. It's the test framework which applies a timeout, considering your test to have failed if it hasn't signalled completion after a certain amount of time.

Given that the test framework is already applying its own timeout logic, I don't think it's especially useful to have wait-for doing its own timeout-handling as well. It would complicate things for little obvious benefit.

Did you have a particular use case in mind where the test framework's timeout logic isn't sufficient? Or are you having particular problems with tests failing to time out, and waiting forever?

[jeaye]

Thanks for the detailed reply and sorry for the delay. There are two use cases for a timeout. For one, the test framework's timeout would suffice, but not for the other.

1. Wait for an event which is never dispatched, due to a logic error (handled by *test-timeout*, though not as gracefully as with a custom timeout)
2. Expect that events should be triggered on a timer, every X ms, and wait no longer than 2X ms before failing the test

Both of these could benefit from a custom timeout in wait-for, and I believe the second one requires it (or the exact same logic done in the test code).

[samroberton]

I have to wonder whether re-frame-test is the right tool to use to write a test like your second use case?

re-frame-test is largely written on the assumption that your test is going to dispatch any events that you expect your application to see manually. Even if in practice your application dispatches events regularly on a timer, I'm not sure that I agree that it would be valuable for the application's re-frame-test tests to use a timer as well: to me, at least, it makes sense for the test to test that your application does the right thing when the events fire, not so much to mimic them firing exactly 2ms apart. For me, that comes back to the idea that part of re-frame's purpose is to allow you to achieve purity (and to write tests which take advantage of that purity) even for code which in practice executes in an impure, side-effect-y world -- such as where event occurrence is dependent on the clock.

Does that make sense? I'm not sure whether I'm fully appreciating why one would want such a thing in a re-frame-test test.

[jeaye]

I have to wonder whether re-frame-test is the right tool to use to write a test like your second use case?

My understanding is that re-frame-test is composed of "utilities for testing re-frame applications." (as per the tagline of the repo) Given that it's not unlikely to be dispatching events on a timer, the timeout seemed like a suitable utility for testing re-frame applications.

not so much to mimic them firing exactly 2ms apart.

I think there's been a misunderstanding, given my terminology. Sorry about that. I'm not looking to verify events are dispatching exactly every 2ms; surely this would be the wrong tool for that, since I'm basically writing a super real-time app at that point and re-frame likely wouldn't be able to handle that very well anyway (depending on what the events do!).

What I meant in saying "a timer, every X ms, and wait no longer than 2X ms" is that X is some number much larger than 1 and more like 1000. If it's been 2000ms and there's been no dispatch, then the app is unresponsive or there's a logic error.

Reading into what you're saying, whether it's 2ms or 2000ms, it doesn't matter much. Correct me if I'm wrong in the interpretation here. I understand your point about not specifically caring about the events and when they fire so much as the effects they cause. You're not the only person with this viewpoint, and I, perhaps, may be the minority in thinking that, if I tell the app to do something, I want to be sure that it does that. If my app state changes, I want to know why it changed, and I only want it to do so exactly when it's supposed to. Monitoring which events are being dispatched, and when, allows me to have this control.

Well, that's my plead. If you're not interested, no problem. :) I can work this logic in on my end. I appreciate the thoughtful replies.

[samroberton]

Urgh, sorry, I didn't help the discussion at all by typing "2ms" when actually I meant to type "2s". Apologies for that.

My understanding is that re-frame-test is composed of "utilities for testing re-frame applications." (as per the tagline of the repo) Given that it's not unlikely to be dispatching events on a timer, the timeout seemed like a suitable utility for testing re-frame applications.

This is a fair comment, so let me dig a bit deeper into it...

re-frame doesn't have a notion of time. If you want to know the current time when you're handling an event, then of course nothing prevents you from using (js/Date.) to find it out -- but, that said, I think it's fair to say that re-frame would encourage you to use a "co-effect" to supply the currently time, so that your event handler remains pure.

The way a re-frame application is structured, all the work gets done in event handlers. Notwithstanding your quote from the tagline of the repo (fair point, of course!), my take on the raison d'être of re-frame-test is to make it easy to test "does my combination of event handlers -- the stuff that makes up the logic of my application -- correctly handle the occurrence of A, then B, then C, then D". And I think it's important that it's the test which is responsible for saying "A happens, then B happens, then C happens, then D happens, and after all that, assert that proposition X is true". If event C happens, in practice in your real app, to be triggered by an event which is scheduled (with js/setTimeout) to occur ever 2s, then that's fine -- but it's not necessarily germane to the test. And in fact I think it limits your test if C is, in the test suite, caused by a js/setTimeout. Because then how would you write a test to assert that A then B then D then C also produces the correct result?

So that's my at-arm's-length argument. To attempt to approach from a more practical viewpoint...

Is there a good reason that the thing that even sets up a (say) 2s js/setTimeout is even called by your test suite? Would it not be possible (and, I would argue, better!) to have the aspects which are related to coordinating timing completely separated from the logic of the application, and only called by the application's entry-point? Obviously you want to test the interaction of the "thing that gets called every 2s" with the rest of your code. But it doesn't seem particularly worthwhile to test that js/setTimeout does indeed schedule something to happen every two seconds. So wouldn't it be better to write tests which can assert "what happens if it gets called before this other thing that might happen around the 2s mark" v "what happens if it gets called after"?

if I tell the app to do something, I want to be sure that it does that. If my app state changes, I want to know why it changed, and I only want it to do so exactly when it's supposed to.

I think this is where we could be talking past one another. I'm sorry, but I don't understand what you're getting at here. Could you elaborate further, perhaps?

Monitoring which events are being dispatched, and when, allows me to have this control.

Hmmm... It's possible that our different approaches to this point could partially answer my question above. The way that I write a re-frame app is that an event is (a) a user interaction (click a button, enter some text in an input field, etc), (b) an AJAX response, or (c) a js/setTimeout that tells the app about the passage of time. These are all occurrences which are new information being communicated to the app -- they are things that have happened which are outside the deterministic control of the app. And so for me, by definition, the arrival of an event in the event queue is not something that could be asserted against: events are "outside" the app -- that is, things that tell the application to do something, not something which you can monitor to see whether the app seems to be doing the right thing. Whereas it sounds like you want to detect the firing of an event in order to assert that internally the app appears to be behaving correctly.

Does that sound to you like an accurate description of where we might be diverging?

[jeaye]

No problem about the 2ms/2s mixup. Thanks, again, for the thoughtful reply. I'm gonna pull some of these out of order, to better fit the flow of my own logic.

Is there a good reason that the thing that even sets up a (say) 2s js/setTimeout is even called by your test suite?

My stance is that this timer should run in the test suite, since it runs in the app. Currently, my test suite is the app, minus the view. Moving any further away from what users see in production is not something I'd like to do.

So wouldn't it be better to write tests which can assert "what happens if it gets called before this other thing that might happen around the 2s mark" v "what happens if it gets called after"?

This is an important thing to test. On my end, it's handled by exercising the timeout ms for those timers, to simulate loads where timers are running behind or running very quickly.

I think it's important that it's the test which is responsible for saying "A happens, then B happens, then C happens, then D happens, and after all that, assert that proposition X is true". If event C happens, in practice in your real app, to be triggered by an event which is scheduled (with js/setTimeout) to occur ever 2s, then that's fine -- but it's not necessarily germane to the test.

I think we disagree fundamentally here. In my tests, as it is in my app, one navigates to different screens, which have their own behaviors. Some screens need to poll for "real-time" data (once a second), while other screens poll ocassionally and some just load their data once. The behavior of this polling, as just an example brought up because of the timers, is crucial into making sure the app behaves properly. If the polling doesn't continue while the app is on that screen, and stop when it's not, then it's either broken (if it doesn't poll) or doing too much work (since there's no need to poll on other screens).

You're not the only person to make this argument to me and I respect that you have a much clearer view than me of re-frame's design from all angles. However, at my core, I disagree with this approach to testing, since I don't think it catches errors which seem to me to be very common; it also strays from the way the user is experiencing the app, which makes it more like a unit test than a functional test. Perhaps the issue here is that I'm using re-frame-test for the wrong category of tests.

things that tell the application to do something, not something which you can monitor to see whether the app seems to be doing the right thing. Whereas it sounds like you want to detect the firing of an event in order to assert that internally the app appears to be behaving correctly.

Yeah, you got it. To me, if you unit test the event handlers, like functions, to ensure they behave just as they should (and you instrument them!), and then you assert that the events are flowing as they should, you can then know that everything is working as it should. Each individual event works, the flow of events works, the app works.

By ignoring the events themselves and focusing on verifying the app state after some events, I truly think you're missing out on a lot of coverage.

[jeaye]

I'm gonna close this, since I think we've made our cases and there isn't quite enough agreement to bring this into re-frame-test. Thanks again for the detailed info and support.