search

minkphp / MinkZombieDriver

Zombie.js driver for Mink framework
41 stars 49 forks source link

Support zombie.js options #154

Closed robballou closed 7 years ago

robballou commented 8 years ago

Here is the start of this functionality. I opted to add this as a last parameter rather than allowing the params or an array just for clarity of the interface. Also I added some "validation" of the options per what is in Zombie's BROWSER_OPTIONS settings.

aik099 commented 8 years ago

Also if you can create some tests for changed code, this would be great. You can manually create ZombieServer and talk to it without using driver class as well.

aik099 commented 8 years ago

@robballou , any updates?

robballou commented 8 years ago

I might have some leeway this week to work on it. Thanks

aik099 commented 8 years ago

Turns out there is a test case covering changed class. @robballou , please also add relevant tests.

stof commented 8 years ago

This should be updated to rely on an environment variable containing a json string

robballou commented 8 years ago

I might have some time this week to work on this. Taking a look at tests right now.

robballou commented 8 years ago

I'm having trouble getting tests to run (just trying in master). If I could get some help with that I can try to write some tests to cover the changes.

Thanks,

Rob

aik099 commented 8 years ago

I'm having trouble getting tests to run

What is the specific error you're having when running phpunit?

robballou commented 8 years ago

Looks like there is an issue with the JS side of things, attaching the phpunit output:

phpunit.txt

I'm using node 5.9.1.

aik099 commented 8 years ago

To summarize the error you're getting from most (if not all) tests is:

Behat\Mink\Exception\DriverException: Error "Unexpected identifier" while executing code: ...

What is Zombie version you're using? I'm on 4.x something. Not sure how to determine which exactly. Try installing 4.0.0 version of Zombie and see if tests pass then.

aik099 commented 8 years ago

And if you're on Windows then you might also get that error.

aik099 commented 8 years ago

@robballou , any update?

robballou commented 8 years ago

Last time I tried, I still had issues running tests but I haven't been able to work on it since April :(

stof commented 7 years ago

For reference, the list of available options is defined at https://github.com/assaf/zombie/blob/v4.3.0/src/index.js#L30

christopher-francisco commented 7 years ago

Will this options include timeout? 5 seconds is just too little

aik099 commented 7 years ago

All options will be passed to Zombie as-as (we won't be storing local copies of allowed options in the driver). So if in Zombie docs there is such options it will be passed.

christopher-francisco commented 7 years ago

What is needed in order to merge this? I can help if you need me to do something (though I'm really new to Zombie in general, been using it for maximum 1 month)

aik099 commented 7 years ago

You need to:

  1. have Zombie installed locally
  2. make sure you can run test suite locally
  3. create new PR as rebased version of this PR, that will also use env variable (according to https://github.com/minkphp/MinkZombieDriver/pull/154#issuecomment-192615761)
  4. add some tests (according to https://github.com/minkphp/MinkZombieDriver/pull/154#issuecomment-179703844)
  5. try to use changes from PR to see if they really set options for Zombie
aik099 commented 7 years ago

Since this PR was created the JS server code was moved to https://github.com/minkphp/MinkZombieDriver/blob/master/bin/mink-zombie-server.js file, that reads server settings from enviroment variables, that are set in https://github.com/minkphp/MinkZombieDriver/blob/614cfc8793bd4661b816be0666db7a2a83636ea9/src/NodeJS/Server.php#L319-L324 when Zombie process is created.

aik099 commented 7 years ago

@chris-fa , you plan on sending PR any time soon?

christopher-francisco commented 7 years ago

@aik099 I'm trying to set up some free time, maybe in the holiday; but I'm not sure whether I might be able to do it quickly, I haven't contributed before to this repo, so I'm not really familiarized with the source code; I'm willing to provide any help I can, but can't guarantee a fast time, sorry :(

aik099 commented 7 years ago

Any help any time is appreciated. Thanks upfront.

christopher-francisco commented 7 years ago

Is anybody maintaining this PR?

aik099 commented 7 years ago

@chris-fa , what do you mean by maintaining? The PR author abandoned this PR more than half a year ago. You told couple of month ago that you might continuing development of this PR and disappeared from the radar as well.

So I guess nobody is writing code for this PR at the moment.

christopher-francisco commented 7 years ago

@aik099 I just realized I wrote on this PR my bad 😂

berliner commented 7 years ago

I would work on this as I need to be able to specify some options for zombiejs.

I'm a little bit confused about where to do that though. Finally I need to have this feature in https://github.com/Behat/MinkZombieDriver which is behind https://github.com/minkphp/MinkZombieDriver by 64 commits at the time of this comment.

Also, there are needed changes in the ZombieFactory of the Behat/MinkExtension, so I suppose that I need to issue a separate PR there?

What I got so far is this: https://github.com/minkphp/MinkZombieDriver/compare/master...berliner:berliner/zombie-options https://github.com/Behat/MinkExtension/compare/master...berliner:berliner/zombie-options

In order to proceed I'd need the following information:

Sorry if any of this is too obvious to you, but I rather ask. I already spent a lot of time figuring out why my tests just don't succeed, digging through pretty cryptic error messages and trying to get my head around on how to pass on zombiejs options when going through mink. The documentation is looking very good, but is actually rather thin for anything that is slightly special.

stof commented 7 years ago

@berliner you don't need this in the https://github.com/Behat/MinkZombieDriver repo, as this is a legacy repo (from the time where download links were not redirected by github and so we forked back the repo after moving it). the source of the behat/mink-zombie-driver composer package is the https://github.com/minkphp/MinkZombieDriver

berliner commented 7 years ago

@stof Thanks for clarifying. It would be good to put a note to that effect on the repo page. It really confused me.

aik099 commented 7 years ago

how to run the included tests in both MinkExtension and MinkZombieDriver

Since Zombie is headless driver you only need to do npm -g install zombie to install Zombie once and then you can run tests as many times as needed, because Zombie process is launched/destroyed on demand.

ideally also how to run this as part of a behat setup, installed using composer require behat/mink-zombie-driver

You'll need to create another PR to MinkExtension so that zombie options can be defined in .behat.yml and then passed along to driver.

berliner commented 7 years ago

@aik099 Thanks for your answers, I need a clarification though: When asking how to run the tests, I actually meant the tests that will be run here on github too, so that I can check these before issuing a PR.

Also, I created a PR for MinkExtension: https://github.com/Behat/MinkExtension/pull/284 I am a little lost with the failed test, it seems the test only failed for PHP 5.3 on something not related to my changes. Can you tell me if this is something I should try to fix or am I right assuming that this is unrelated?

aik099 commented 7 years ago

When asking how to run the tests, I actually meant the tests that will be run here on github too, so that I can check these before issuing a PR.

What you see on GitHub is result of running phpunit command on Travis CI servers. You can run same phpunit command inside your GitHub repo clone considering you have PHPUnit installed.

Also, I created a PR for MinkExtension: Behat/MinkExtension#284 I am a little lost with the failed test, it seems the test only failed for PHP 5.3 on something not related to my changes. Can you tell me if this is something I should try to fix or am I right assuming that this is unrelated?

Nothing needs to be fixed. I've responded in that PR as well with more details.

berliner commented 7 years ago

@aik099 Thanks, that's been sufficiently clear. I am able to run the tests now using phpspec.

aik099 commented 7 years ago

PhpSpec? I thought mink tests are written in PHPUnit, but Behat tests (and possible MinkExtension tests) are written in PhpSpec.

berliner commented 7 years ago

PhpSpec? I thought mink tests are written in PHPUnit, but Behat tests (and possible MinkExtension tests) are written in PhpSpec.

Sorry to confuse you. I was looking at MinkExtension at the moment, which uses phpspec. Your previous answer made it clear for me that I simply needed to look at the travis logs to find out how to run the tests for the project, which is what I did for MinkExtension as I have an PR open there. I'll be looking at MinkZombieDriver next, which seems to be using phpunit as you said.

aik099 commented 7 years ago

Replaced by #183.