Skip to content
This repository was archived by the owner on Jan 9, 2025. It is now read-only.

Add support for zombie options #183

Merged
aik099 merged 7 commits into from
Jul 18, 2017
Merged

Add support for zombie options #183

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f2b6334
Add support for zombiejs options
Jun 9, 2017
61e5bcb
Merge branch 'master' into berliner/zombie-options
Jun 23, 2017
147cc68
Updated tests with asserts for zombie options
Jun 23, 2017
066669f
Add argument validation and associated test for zombie options
Jun 23, 2017
d98cad7
Changed order of test functions
Jun 23, 2017
44c9adf
Use type hint instead of checking options argument, remove test case
Jun 23, 2017
6d6bf10
Change visibility of member variable
Jul 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion bin/mink-zombie-server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ var pointers = [];
var buffer = '';
var host = process.env.HOST || '127.0.0.1';
var port = process.env.PORT || 8124;
var options = process.env.OPTIONS ? JSON.parse(process.env.OPTIONS) : {};

var zombieVersionCompare = function (v2, op) {
var version_compare = function (v1, v2, operator) {
Expand Down Expand Up @@ -108,7 +109,7 @@ net.createServer(function (stream) {

stream.on('end', function () {
if (browser == null) {
browser = new zombie();
browser = new zombie(options);

// Clean up old pointers
pointers = [];
Expand Down
40 changes: 39 additions & 1 deletion src/NodeJS/Server.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ abstract class Server
*/
protected $threshold;

/**
* @var array
*/
protected $options;
Copy link
Copy Markdown
Member

@stof stof Jun 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use private visibility for the new property (as it has both a getter and a setter which are public, there is no benefit making this accessible through inheritance, and protected stuff makes it harder to handle backward compatibility)

Copy link
Copy Markdown
Contributor Author

@berliner berliner Jun 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stof I tried to follow what's already there and all the other properties are protected while having setters and getters.

Copy link
Copy Markdown
Member

@aik099 aik099 Jul 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@berliner , please make this change. I guess that's last thing before this PR is merged.

Copy link
Copy Markdown
Contributor Author

@berliner berliner Jul 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aik099 Done, though I still feel that this doesn't belong into this PR.


/**
* @var string The full path to the NodeJS modules directory.
*/
Expand All @@ -69,14 +74,16 @@ abstract class Server
* @param string $serverPath Path to server script
* @param int $threshold Threshold value in micro seconds
* @param string $nodeModulesPath Path to node_modules directory
* @param array $options Options array for zombiejs
*/
public function __construct(
$host = '127.0.0.1',
$port = 8124,
$nodeBin = null,
$serverPath = null,
$threshold = 2000000,
$nodeModulesPath = ''
$nodeModulesPath = '',
$options = array()
) {
if (null === $nodeBin) {
$nodeBin = 'node';
Expand All @@ -97,6 +104,7 @@ public function __construct(

$this->setServerPath($serverPath);
$this->setThreshold($threshold);
$this->setOptions($options);
}

/**
Expand Down Expand Up @@ -273,6 +281,32 @@ public function getThreshold()
return $this->threshold;
}

/**
* Setter options value
*
* @param array $options Options array
*
* @throws \LogicException When server is already running.
*/
public function setOptions(array $options)
{
if ($this->isRunning()) {
throw new \LogicException('Unable to change options of a running server.');
}

$this->options = $options;
}

/**
* Getter options value
*
* @return array Options array
*/
public function getOptions()
{
return $this->options;
}

/**
* Getter process object
*
Expand Down Expand Up @@ -323,6 +357,10 @@ public function start(Process $process = null)
$processBuilder->setEnv('NODE_PATH', $this->nodeModulesPath);
}

if (!empty($this->options)) {
$processBuilder->setEnv('OPTIONS', json_encode($this->options));
}

$process = $processBuilder->getProcess();
}
$this->process = $process;
Expand Down
36 changes: 26 additions & 10 deletions tests/Custom/NodeJS/ServerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ protected function doEvalJS(Connection $conn, $str, $returnType = 'js')
protected function getServerScript()
{
return <<<JS
var zombie = require('zombie')
, host = process.env.HOST
, port = process.env.PORT;
var zombie = require('zombie')
, host = process.env.HOST
, port = process.env.PORT
, options = process.env.OPTIONS ? JSON.parse(process.env.OPTIONS) : {};
JS;
}

Expand Down Expand Up @@ -58,11 +59,13 @@ public function testCreateServerWithDefaults()
$this->assertEquals('/path/to/server', $server->getServerPath());
$this->assertEquals(2000000, $server->getThreshold());
$this->assertEquals('', $server->getNodeModulesPath());
$this->assertEquals(array(), $server->getOptions());

$expected = <<<JS
var zombie = require('zombie')
, host = process.env.HOST
, port = process.env.PORT;
var zombie = require('zombie')
, host = process.env.HOST
, port = process.env.PORT
, options = process.env.OPTIONS ? JSON.parse(process.env.OPTIONS) : {};
JS;
$this->assertEquals($expected, $server->serverScript);
}
Expand Down Expand Up @@ -90,7 +93,8 @@ public function testCreateCustomServer()
null,
null,
5000000,
'../../'
'../../',
array('waitDuration' => '15s')
);

$this->assertEquals('123.123.123.123', $server->getHost());
Expand All @@ -99,11 +103,13 @@ public function testCreateCustomServer()
$this->assertEquals('/path/to/server', $server->getServerPath());
$this->assertEquals(5000000, $server->getThreshold());
$this->assertEquals('../../', $server->getNodeModulesPath());
$this->assertEquals(array('waitDuration' => '15s'), $server->getOptions());

$expected = <<<JS
var zombie = require('zombie')
, host = process.env.HOST
, port = process.env.PORT;
var zombie = require('zombie')
, host = process.env.HOST
, port = process.env.PORT
, options = process.env.OPTIONS ? JSON.parse(process.env.OPTIONS) : {};
JS;
$this->assertEquals($expected, $server->serverScript);
}
Expand Down Expand Up @@ -158,6 +164,16 @@ public function testSetThresholdOnRunningServer()
$server->setThreshold('test');
}

/**
* @expectedException \LogicException
* @expectedExceptionMessage Unable to change options of a running server.
*/
public function testSetOptionsOnRunningServer()
{
$server = $this->getRunningServer();
$server->setOptions(array('waitDuration' => '15s'));
}

/**
* @group legacy
*/
Expand Down