Open gentoo90 opened 8 years ago
Sorry for my delay. Thanks for taking the time to put this together. The Travis build is broken so I think this patch probably has a bug in it?
Why do you need the page.readyEventOccured = true;
part of the patch? I don't understand why using console.log
is the best approach. Are you just trying to get around the security boundary?
What I'd do instead is having doInject
do a page.evaluate()
call to see if an attribute on window
is set. If it's not, then go back to waitForReady until the attribute is done. You can define a callback function before the page rendering using page.evaluate()
as well. Then use that instead of the console.log
you're relying on.
Basically, I'm asking for you to add an API like window.dpxdtReady()
within the page and a config option that says to wait for that function. Does that make sense?
I launched travis build again on the same commit and tests succeeded. The test in pull request failed with
self.assertEquals(work_queue.WorkQueue.DONE, found.status)
AssertionError: 'done' != u'live'
so probably some task from the queue didn't finish in time.
I used console.log
to trigger readyEvent
because pages may already have some log messages like page loaded
and this approach will allow to test them without modifying their code or injecting anything.
I added window.dpxdtReady
field as a possible trigger for page ready event.
But I still would like to be able to use console.log()
, from pages own code for example.
@bslatkin could you merge both of this features?
If
config.readyEvent
is set,phantomjs
will wait for exactly thisconsole.log
message before taking a screenshot.config.delay
is applied before taling a screenshot (afterreadyEvent
if it's set).So with config
phantomjs
will wait for one second after the stringcaptureScreen
is logged to the console.Both features are from BackstopJS.
Fixes #123.