assaf / zombie

Insanely fast, full-stack, headless browser testing using node.js
http://zombie.js.org/
MIT License
5.65k stars 520 forks source link

browser.waitFor > 5000 ignored #263

Closed sbange closed 12 years ago

sbange commented 12 years ago

Hi,

I tried to use waitFor with values way beyond 5000. Initial calls after system startup take that much time because of lazy initializations in backend systems. Very high timeouts did not work so I searched and found that there is a fixed value of 5000 in eventloop.coffee in wait (currently line 159).

Would it be possible to either increase this value or have it as an additional option in options?

Regards, Sven.

assaf commented 12 years ago

Increasing the value would just make "not doing anything useful" tests take longer to complete. Adding another option would just complicate the API and documentation. Using two wait calls in sequence will give you twice the wait time, without having to learn any new API changes, or compromise with other people's preferred timeout.

joeyAghion commented 12 years ago

I had a related experience using browser.wait(condition, callback). The callback was called even though the condition function never returned true (after exhausting eventloop's 5000 ms timeout). Is this by design? The result, in our case, was tests "passing" despite intermediate assertions failing.

assaf commented 12 years ago

The callback will not wait forever. I don't think being stuck forever helps.

joeyAghion commented 12 years ago

Agree. I'm just thinking that the callback should be called with an error if the condition timed out without being satisfied.

ghost commented 12 years ago

/**

I get this::

first fire now second should have waited 5 seconds third should have waited another 5 seconds

node.js:201 throw e; // process.nextTick error, or 'error' event on first tick ^ AssertionError: "" == "This is the new Video page" at /Users/web2guru/nodesites/play/zombie/homepage_and_basic_login.js:95:12 at /Users/web2guru/nodesites/play/node_modules/zombie/lib/zombie/browser.coffee:200:30 at Array.0 (/Users/web2guru/nodesites/play/node_modules/zombie/lib/zombie/eventloop.coffee:160:18) at EventEmitter._tickCallback (node.js:192:40)

I am using version: "zombie": "0.12.13" and node -v 0.6.8, running an express app (jade). A partial is being loaded after the navigation link click. The partial has a getScript (jquery) and that then returns and may manipulate the HTML further. Hence wanting to wait for a second or two after the link is clicked. In the example I have used 5s and a further 5s but when running the test it doesn't wait at all as far as I can tell. When I visit the page normally I see the h1, it just doesn't find it in this test becuase I presume there is no delay kicking in.

What am I doing wrong?

sbange commented 12 years ago

Why shouldn't the client decide how long browser.waitFor should really wait? As of now every value above 5000 will get capped to 5000 by eventloops fixed timeout. I can use browser(..., {waitFor: 150000}) but this will not result in 150000 ms wait but in 5000 ms wait. If I want a single call to have a longer timeout why shouldn't I? No need for API changes no need to increase the fixed timeout. Again: I'm talking about the option "waitFor" handed to browser.visit, I know I can hand over a timeout instead of options, but I want to use both "runScripts" and "waitFor".

assaf commented 12 years ago

Short version is, waiting has to eventually terminate, otherwise people will be wondering why Zombie is stuck forever and never returning.

Having one sane limit for default behavior and some way to over-ride it for special case is acceptable. Suggest a pull request and I'll look at it.

sbange commented 12 years ago

I understand the endless waiting issue, will try to come up with a sane suggestion.

ghost commented 12 years ago

I am struggling to get the wait to work at all. In my code above there is no waiting; it disregards any times that I set and fires immediately. Are other people able to get the browser.wait to work?

assaf commented 12 years ago

It won't wait if there's nothing to wait for.

ghost commented 12 years ago

After a link is clicked on my page, a jquery $.load is triggered and when it returns i.e populates (overwrites) a section of html. This new HTML is what I am testing for, so I need the page to wait upto 5 seconds after the person clicks the link and give the $.load chance to write the new HTML into the page.

I need some way to pause the test, is there a work around you can think of? Would a timeout work within the test itself?

mgan59 commented 12 years ago

@vainrobot I'm having a similar issue in the sense my dom updates and zombiejs has no idea the event occured (I'm replacing the 'src' on an image tag). Think this is a separate issue from .wait as @assaf said there is nothing to wait for. I just opened up the source code today and my best guess are these DOM events aren't in the eventLoop so they are never deferred to in the .wait. So maybe this should be a different issue-thread on whether or not there should be an interace to register custom-dom events for onload. Or is there one and I don't know?

I see all sides of this argument. I think it caused some confusion on my part as I had some async/race-conditions in my tests that I could not figure out, even after jacking my waitFor up to 20000. I had missed the one the block of text in the documentation that mentioned the 5000 default-cap. It is unfortunate that my app takes longer than 5 seconds, but it is a dev environment on my laptop with lots going on so it is a little slow. I'd like to be able to bump this value up just abit to give myself some cushion.