From ee7808b7bf0556d396f775155203a567dfab87eb Mon Sep 17 00:00:00 2001 From: Nick Liu Date: Wed, 15 Aug 2018 23:04:59 -0500 Subject: [PATCH] Major improvements to Deployer system (backwards-incompatible) Deployers, a concept unique to this repository, are now more separated from Codeception modules. This commit adds NoopDeployer, LocalDeployer, and SFTPDeployer to address the three deployment target types in use by testers today. The changes are backwards-incompatible because the structure of config.sample.yml has changed, and all testers need to change their config.yml or config.local.yml to continue testing. The reason for this change is that the section "manual" no longer makes sense now that Deployers are on a spectrum of automation levels. The subsections under "manual" have been broken out into the root level. The "db_dump" section has been merged into the new "db" root section. There is a new "fs" root section used by the SFTP Deployer. Other changes, enhancements, and bugfixes: * cPanelDeployer no longer downgrades to "manual" mode when credentials are missing or an unsupported component is requested. It now throws an exception. * Deployer::unlinkAppFile() was implemented for acceptance tests out of necessity because the app requires a configuration file to be deleted before re-running the app's installer. * If a Deployer subclass does not implement the unlinkAppFile() method, tests that depend on the method will be skipped gracefully. * DeployerFactory now has a better autoload mechanism. * A logical error in lib/config.php prevented missing nested array items from using their default values. * The Base Helper no longer pointlessly caches the DelayedDb module * _bootstrap.php serializes the config.yml params into a global constant so that the DeployerFactory can freely access the information. --- codeception.yml | 17 +-- config.sample.yml | 90 ++++++++++----- e107 | 2 +- lib/config.php | 2 +- lib/deployers/Deployer.php | 25 +++- lib/deployers/DeployerFactory.php | 42 +++++++ lib/deployers/LocalDeployer.php | 18 +++ .../{DummyDeployer.php => NoopDeployer.php} | 2 +- lib/deployers/SFTPDeployer.php | 107 ++++++++++++++++++ lib/deployers/cPanelDeployer.php | 37 ++---- tests/_bootstrap.php | 5 +- tests/_support/Helper/Acceptance.php | 7 -- tests/_support/Helper/Base.php | 11 +- tests/_support/Helper/DeployerFactory.php | 37 ------ tests/_support/Helper/E107Base.php | 4 +- tests/acceptance.suite.yml | 2 +- 16 files changed, 283 insertions(+), 125 deletions(-) create mode 100644 lib/deployers/DeployerFactory.php create mode 100644 lib/deployers/LocalDeployer.php rename lib/deployers/{DummyDeployer.php => NoopDeployer.php} (76%) create mode 100644 lib/deployers/SFTPDeployer.php delete mode 100644 tests/_support/Helper/DeployerFactory.php diff --git a/codeception.yml b/codeception.yml index 711759c1a..a69896d7f 100644 --- a/codeception.yml +++ b/codeception.yml @@ -19,16 +19,9 @@ extensions: - Codeception\Extension\RunFailed modules: enabled: - - \Helper\DeployerFactory: - secrets: - cpanel: - enabled: '%cpanel.enabled%' - hostname: '%cpanel.hostname%' - username: '%cpanel.username%' - password: '%cpanel.password%' - \Helper\DelayedDb: - dsn: 'mysql:host=%manual.db.host%;port=%manual.db.port%;dbname=%manual.db.dbname%' - user: '%manual.db.user%' - password: '%manual.db.password%' - populate: '%db_dump.enabled%' - dump: '%db_dump.path%' + dsn: 'mysql:host=%db.host%;port=%db.port%;dbname=%db.dbname%' + user: '%db.user%' + password: '%db.password%' + populate: '%db.populate%' + dump: '%db.dump_path%' diff --git a/config.sample.yml b/config.sample.yml index eb89499f8..88d01ae43 100644 --- a/config.sample.yml +++ b/config.sample.yml @@ -4,24 +4,24 @@ # Absolute path begins with "/"; relative path does not begin with "/" app_path: 'e107/' -# Configure this section to customize the database populator -db_dump: +# Which deployer to use for acceptance tests. Options: +# +# 'none' +# Dummy deployer that does nothing. Tests that depend on a deployer will fail. +# 'local' +# Use this if the acceptance test web server directly serves files from "app_path". +# Configure the "url" and "db" sections. +# 'sftp' +# Deploys the files in "app_path" to an SFTP account. +# Configure the "url", "db", and "fs" sections. +# 'cpanel' +# Deploys the files in "app_path" to a cPanel account's main domain. +# Configure the "cpanel" section. +deployer: 'local' - # If set to true, the populator will populate the database with the dump specified in the "path" key - # If set to false, the test database needs to be set up separately - # Affects all modes of deployment - enabled: true - - # Path (absolute or relative) to the database dump of a testable installation of the app - # Absolute path begins with "/"; relative path does not begin with "/" - path: 'tests/_data/e107_v2.1.8.sample.sql' - -# Configure this section for automated test deployments to cPanel +# Configure this section for fully automated test deployments to cPanel cpanel: - # If set to true, this section takes precedence over the "manual" section. - enabled: false - # cPanel domain without the port number hostname: '' @@ -32,25 +32,53 @@ cpanel: password: '' -# Configure this section for manual test deployments -manual: +# URL (with trailing slash) at which the app can be reached for acceptance tests +url: 'http://set-this-to-your-acceptance-test-url.local/' - # URL to the app that you deployed manually; needed for acceptance tests - url: 'http://set-this-if-running-acceptance-tests-manually.local' +# Only MySQL/MariaDB is supported +db: - # Only MySQL/MariaDB is supported - db: - # Hostname or IP address; use 'localhost' for a local server - host: 'set-this-if-running-tests-manually.local' + # Hostname or IP address; use 'localhost' for a local server + host: 'set-this-to-your-test-database-hostname.local' - # Port number of the server - port: '3306' + # Port number of the server + port: '3306' - # Database name; must exist already - dbname: 'e107' + # Database name; must exist already + dbname: 'e107' - # Username; must exist already - user: 'root' + # Username; must exist already + user: 'root' - # Password; set to blank string for no password - password: '' + # Password; set to blank string for no password + password: '' + + # If set to true, the database populator will populate the database with the dump specified in the "dump_path" key + # If set to false, the test database needs to be set up separately + # Affects all tests and modes of deployment + populate: true + + # Path (absolute or relative) to the database dump of a testable installation of the app + # Absolute path begins with "/"; relative path does not begin with "/" + dump_path: 'tests/_data/e107_v2.1.8.sample.sql' + +# Configure this section for deployers that need file upload configuration +fs: + + # Hostname or IP address to the remote destination + host: '' + + # Port number of the file transfer server + port: '22' + + # Username used for the file transfer + user: '' + + # Path to the private key of the user. Takes precedence over "fs.password" + privkey_path: '' + + # Password of the file transfer user. Ignored if "fs.privkey_path" is specified + password: '' + + # Absolute path to where the remote web server serves "url" + path: '' \ No newline at end of file diff --git a/e107 b/e107 index e2460e0b3..73fbe980a 160000 --- a/e107 +++ b/e107 @@ -1 +1 @@ -Subproject commit e2460e0b3aa2fe562c2484b5742365a72fa334c1 +Subproject commit 73fbe980a432c4520ea99b21f11a16a5fd88110b diff --git a/lib/config.php b/lib/config.php index d5f8d58f5..3933179e3 100644 --- a/lib/config.php +++ b/lib/config.php @@ -12,7 +12,7 @@ foreach ([ { $absolute_config_path = codecept_root_dir() . '/' . $config_filename; if (file_exists($absolute_config_path)) - $params = array_merge($params, Yaml::parse(file_get_contents($absolute_config_path))); + $params = array_replace_recursive($params, Yaml::parse(file_get_contents($absolute_config_path))); } return $params; diff --git a/lib/deployers/Deployer.php b/lib/deployers/Deployer.php index 861ad3ba8..a376e395d 100644 --- a/lib/deployers/Deployer.php +++ b/lib/deployers/Deployer.php @@ -5,6 +5,23 @@ abstract class Deployer abstract public function start(); abstract public function stop(); + protected $params; + + public function __construct($params = []) + { + $this->params = $params; + } + + protected static function println($text = '') + { + codecept_debug($text); + + //echo("${text}\n"); + + //$prefix = debug_backtrace()[1]['function']; + //echo("[\033[1m${prefix}\033[0m] ${text}\n"); + } + protected $components = array(); /** @@ -15,6 +32,12 @@ abstract class Deployer $this->components = $components; } + public function unlinkAppFile($relative_path) + { + throw new \PHPUnit\Framework\SkippedTestError("Test wants \"$relative_path\" to be deleted from the app, ". + "but the configured deployer ".get_class($this)." is not capable of doing that."); + } + /** * Methods not implemented * @@ -24,6 +47,6 @@ abstract class Deployer */ public function __call($method_name, $arguments) { - return null; + throw new BadMethodCallException(get_class($this)."::$method_name is not implemented"); } } \ No newline at end of file diff --git a/lib/deployers/DeployerFactory.php b/lib/deployers/DeployerFactory.php new file mode 100644 index 000000000..0e8afa157 --- /dev/null +++ b/lib/deployers/DeployerFactory.php @@ -0,0 +1,42 @@ +components)) + { + $this->start_fs(); + } + } + + private function getFsParams() + { + return $this->params['fs']; + } + + private function generateSshpassPrefix() + { + if (empty($this->getFsParam('privkey_path')) && + !empty($this->getFsParam('password'))) + { + return 'sshpass -p '.escapeshellarg($this->getFsParam('password')).' '; + } + return ''; + } + + private function getFsParam($key) + { + return $this->getFsParams()[$key]; + } + + private function generateRsyncRemoteShell() + { + $prefix = 'ssh -p '.escapeshellarg($this->getFsParam('port')); + if (!empty($this->getFsParam('privkey_path'))) + return $prefix.' -i ' . escapeshellarg($this->getFsParam('privkey_path')); + else + return $prefix; + } + + private static function runCommand($command, &$stdout = null, &$stderr = null) + { + $descriptorSpec = [ + 1 => ['pipe', 'w'], + 2 => ['pipe', 'w'], + ]; + $pipes = []; + self::println("Running this command…:"); + self::println($command); + $resource = proc_open($command, $descriptorSpec, $pipes, APP_PATH); + $stdout = stream_get_contents($pipes[1]); + $stderr = stream_get_contents($pipes[2]); + self::println("---------- stdout ----------"); + self::println(trim($stdout)); + self::println("---------- stderr ----------"); + self::println(trim($stderr)); + self::println("----------------------------"); + foreach ($pipes as $pipe) + { + fclose($pipe); + } + return proc_close($resource); + } + + public function stop() + { + self::println("=== SFTP Deployer – Tear Down ==="); + } + + public function unlinkAppFile($relative_path) + { + self::println("Deleting file \"$relative_path\" from deployed test location…"); + $fs_params = $this->getFsParams(); + $command = $this->generateSshpassPrefix(). + $this->generateRsyncRemoteShell(). + " ".escapeshellarg("{$fs_params['user']}@{$fs_params['host']}"). + " ".escapeshellarg("rm -v " . escapeshellarg(rtrim($fs_params['path'], '/')."/$relative_path")); + $retcode = self::runCommand($command); + if ($retcode === 0) + { + self::println("Deleted file \"$relative_path\" from deployed test location"); + } + else + { + self::println("No such file to delete: \"$relative_path\""); + } + } + + private function start_fs() + { + $fs_params = $this->getFsParams(); + $fs_params['path'] = rtrim($fs_params['path'], '/') . '/'; + $command = $this->generateSshpassPrefix() . + 'rsync -e ' . + escapeshellarg($this->generateRsyncRemoteShell()) . + ' --delete -avzHXShs ' . + escapeshellarg(rtrim(APP_PATH, '/') . '/') . ' ' . + escapeshellarg("{$fs_params['user']}@{$fs_params['host']}:{$fs_params['path']}"); + $retcode = self::runCommand($command); + if ($retcode !== 0) { + throw new Exception("SFTP deployment failed. Run with --debug to see stdout and stderr."); + } + } +} \ No newline at end of file diff --git a/lib/deployers/cPanelDeployer.php b/lib/deployers/cPanelDeployer.php index 89a566760..d3f1172d1 100644 --- a/lib/deployers/cPanelDeployer.php +++ b/lib/deployers/cPanelDeployer.php @@ -17,9 +17,10 @@ class cPanelDeployer extends Deployer protected $domain; private $skip_mysql_remote_hosts = false; - function __construct($credentials) + function __construct($params = []) { - $this->credentials = $credentials; + parent::__construct($params); + $this->credentials = $params['cpanel']; } public function start() @@ -31,8 +32,7 @@ class cPanelDeployer extends Deployer !$creds['username'] || !$creds['password']) { - self::println("Cannot deploy cPanel environment because credentials are missing. Falling back to manual mode…"); - return false; + throw new Exception("Cannot deploy cPanel environment because credentials are missing."); } $this->prepare(); @@ -42,8 +42,7 @@ class cPanelDeployer extends Deployer $method = "prepare_${component}"; if (!method_exists($this, $method)) { - self::println("Unsupported component \"${component}\" requested. Falling back to manual mode…"); - return false; + throw new Exception("Unsupported component \"${component}\" requested."); } } foreach ($this->components as $component) @@ -51,18 +50,6 @@ class cPanelDeployer extends Deployer $method = "prepare_${component}"; $this->$method(); } - - return true; - } - - private static function println($text = '') - { - codecept_debug($text); - - //echo("${text}\n"); - - //$prefix = debug_backtrace()[1]['function']; - //echo("[\033[1m${prefix}\033[0m] ${text}\n"); } private function prepare() @@ -288,6 +275,13 @@ class cPanelDeployer extends Deployer return "http://".$this->domain."/".$this->run_id."/"; } + public function unlinkAppFile($relative_path) + { + self::println("Deleting file \"$relative_path\" from deployed test location…"); + $this->cPanel->api2->Fileman->fileop(['op' => 'unlink', + 'sourcefiles' => self::TARGET_RELPATH.$this->run_id."/".$relative_path]); + } + private function prepare_db() { $cPanel = $this->cPanel; @@ -373,11 +367,4 @@ class cPanelDeployer extends Deployer return $tmp_file; } - - public function unlinkAppFile($relative_path) - { - self::println("Deleting file \"$relative_path\" from deployed test location…"); - $this->cPanel->api2->Fileman->fileop(['op' => 'unlink', - 'sourcefiles' => self::TARGET_RELPATH.$this->run_id."/".$relative_path]); - } } diff --git a/tests/_bootstrap.php b/tests/_bootstrap.php index 2950c7b79..b635f8686 100644 --- a/tests/_bootstrap.php +++ b/tests/_bootstrap.php @@ -1,6 +1,8 @@ deployer->unlinkAppFile("e107_config.php"); - - // Local Environment - if (file_exists(APP_PATH."/e107_config.php")) - { - unlink(APP_PATH."/e107_config.php"); - } } } diff --git a/tests/_support/Helper/Base.php b/tests/_support/Helper/Base.php index b3bd13d24..53aaf5369 100644 --- a/tests/_support/Helper/Base.php +++ b/tests/_support/Helper/Base.php @@ -1,5 +1,6 @@ db ?: $this->db = $this->getModule('\Helper\DelayedDb'); + return $this->getModule('\Helper\DelayedDb'); } public function getBrowserModule() @@ -23,7 +22,7 @@ abstract class Base extends \Codeception\Module public function _beforeSuite($settings = array()) { - $this->deployer = $this->getModule('\Helper\DeployerFactory')->create(); + $this->deployer = \DeployerFactory::create(); $this->deployer->setComponents($this->deployer_components); $this->deployer->start(); @@ -31,8 +30,10 @@ abstract class Base extends \Codeception\Module foreach ($this->getModules() as $module) { - if (get_class($module) !== get_class($this)) + if (!$module instanceof $this) + { $module->_beforeSuite(); + } } } diff --git a/tests/_support/Helper/DeployerFactory.php b/tests/_support/Helper/DeployerFactory.php deleted file mode 100644 index 00da9a411..000000000 --- a/tests/_support/Helper/DeployerFactory.php +++ /dev/null @@ -1,37 +0,0 @@ -createFromSecrets($this->config['secrets']); - } - - /** - * @param $secrets - * @return \Deployer - */ - public function createFromSecrets($secrets) - { - $deployer = new \DummyDeployer(); - if ($secrets['cpanel']['enabled'] === '1') - { - $deployer = new \cPanelDeployer($secrets['cpanel']); - } - return $deployer; - } - -} diff --git a/tests/_support/Helper/E107Base.php b/tests/_support/Helper/E107Base.php index cf15b301f..7eac93b16 100644 --- a/tests/_support/Helper/E107Base.php +++ b/tests/_support/Helper/E107Base.php @@ -55,8 +55,8 @@ abstract class E107Base extends Base { $descriptorspec = [ 1 => ['pipe', 'w'], - 2 => ['pipe', 'w'], - ]; + 2 => ['pipe', 'w'], + ]; $pipes = []; $resource = proc_open('git clean -fdx', $descriptorspec, $pipes, APP_PATH); //$stdout = stream_get_contents($pipes[1]); diff --git a/tests/acceptance.suite.yml b/tests/acceptance.suite.yml index 5633ef875..8bc2afcc5 100644 --- a/tests/acceptance.suite.yml +++ b/tests/acceptance.suite.yml @@ -10,5 +10,5 @@ coverage: modules: enabled: - PhpBrowser: - url: '%manual.url%' + url: '%url%' - \Helper\Acceptance: