Skip to content

Define IS_WINDOWS in the test runner#4866

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
villfa wants to merge 1 commit into from
Closed

Define IS_WINDOWS in the test runner #4866

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 11 additions & 9 deletions run-tests.php
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -57,6 +57,8 @@ function main()
// Parallel testing
global $workers, $workerID;

define('IS_WINDOWS', substr(PHP_OS, 0, 3) == "WIN");

@KalleZKalleZOct 27, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I personally think this is cleaner:

PHP_OS_FAMILY == 'Windows'

The substr() call has always been rather redundant for Windows unless you wanted to differentiate between Windows NT and Windows 9x series.

Either way I'm fine with this change overall

@nikicnikicOct 28, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

PHP_OS_FAMILY is in 7.2, but run-tests is currently compatible with 7.0.

@GirgiasGirgiasOct 28, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

PHP_OS_FAMILY is in 7.2, but run-tests is currently compatible with 7.0.

As this is targeting master shouldn't this be alright? Or is run-tests used version agnostic?

@nikicnikicOct 28, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Context: #3981

We can bump the minimum version requirement, I'm just saying that this is the status quo.


$workerID = 0;
if (getenv("TEST_PHP_WORKER")) {
$workerID = intval(getenv("TEST_PHP_WORKER"));
Expand DownExpand Up@@ -135,7 +137,7 @@ function main()
}
}
//
if ((substr(PHP_OS, 0, 3) == "WIN") && empty($environment["SystemRoot"])) {
if (IS_WINDOWS && empty($environment["SystemRoot"])) {
$environment["SystemRoot"] = getenv("SystemRoot");
}

Expand DownExpand Up@@ -175,7 +177,7 @@ function main()
}

if (!getenv('TEST_PHPDBG_EXECUTABLE')) {
if (!strncasecmp(PHP_OS, "win", 3) && file_exists(dirname($php) . "/phpdbg.exe")) {
if (IS_WINDOWS && file_exists(dirname($php) . "/phpdbg.exe")) {
$phpdbg = realpath(dirname($php) . "/phpdbg.exe");
} elseif (file_exists(dirname($php) . "/../../sapi/phpdbg/phpdbg")) {
$phpdbg = realpath(dirname($php) . "/../../sapi/phpdbg/phpdbg");
Expand DownExpand Up@@ -659,7 +661,7 @@ function main()
}

if (strlen($conf_passed)) {
if (substr(PHP_OS, 0, 3) == "WIN") {
if (IS_WINDOWS) {
$pass_options .= " -c " . escapeshellarg($conf_passed);
} else {
$pass_options .= " -c '" . realpath($conf_passed) . "'";
Expand DownExpand Up@@ -977,7 +979,7 @@ function save_or_mail_results()
$failed_tests_data .= "OS:\n" . PHP_OS . " - " . php_uname() . "\n\n";
$ldd = $autoconf = $sys_libtool = $libtool = $compiler = 'N/A';

if (substr(PHP_OS, 0, 3) != "WIN") {
if (!IS_WINDOWS) {
/* If PHP_AUTOCONF is set, use it; otherwise, use 'autoconf'. */
if (getenv('PHP_AUTOCONF')) {
$autoconf = shell_exec(getenv('PHP_AUTOCONF') . ' --version');
Expand DownExpand Up@@ -1930,7 +1932,7 @@ function run_test($php, $file, $env)
if (array_key_exists('CGI', $section_text) || !empty($section_text['GET']) || !empty($section_text['POST']) || !empty($section_text['GZIP_POST']) || !empty($section_text['DEFLATE_POST']) || !empty($section_text['POST_RAW']) || !empty($section_text['PUT']) || !empty($section_text['COOKIE']) || !empty($section_text['EXPECTHEADERS'])) {
if (isset($php_cgi)) {
$php = $php_cgi . ' -C ';
} else if (!strncasecmp(PHP_OS, "win", 3) && file_exists(dirname($php) . "/php-cgi.exe")) {

@GirgiasGirgiasOct 27, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I suppose the usage of strncasecmp() is irrelevant compared to substr() but it is weird seeing it.

} else if (IS_WINDOWS && file_exists(dirname($php) . "/php-cgi.exe")) {
$php = realpath(dirname($php) . "/php-cgi.exe") . ' -C ';
} else {
if (file_exists(dirname($php) . "/../../sapi/cgi/php-cgi")) {
Expand DownExpand Up@@ -2086,7 +2088,7 @@ function run_test($php, $file, $env)
$ext_dir = `$php $pass_options $extra_options $ext_params -d display_errors=0 -r "echo ini_get('extension_dir');"`;
$extensions = preg_split("/[\n\r]+/", trim($section_text['EXTENSIONS']));
$loaded = explode(",", `$php $pass_options $extra_options $ext_params -d display_errors=0 -r "echo implode(',', get_loaded_extensions());"`);
$ext_prefix = substr(PHP_OS, 0, 3) === "WIN" ? "php_" : "";
$ext_prefix = IS_WINDOWS ? "php_" : "";
foreach ($extensions as $req_ext) {
if (!in_array($req_ext, $loaded)) {
if ($req_ext == 'opcache') {
Expand DownExpand Up@@ -2125,7 +2127,7 @@ function run_test($php, $file, $env)
if (trim($section_text['SKIPIF'])) {
show_file_block('skip', $section_text['SKIPIF']);
save_text($test_skipif, $section_text['SKIPIF'], $temp_skipif);
$extra = substr(PHP_OS, 0, 3) !== "WIN" ?
$extra = !IS_WINDOWS ?
"unset REQUEST_METHOD; unset QUERY_STRING; unset PATH_TRANSLATED; unset SCRIPT_FILENAME; unset REQUEST_METHOD;" : "";

if ($valgrind) {
Expand DownExpand Up@@ -2480,7 +2482,7 @@ function run_test($php, $file, $env)
$clean_params = array();
settings2array($ini_overwrites, $clean_params);
$clean_params = settings2params($clean_params);
$extra = substr(PHP_OS, 0, 3) !== "WIN" ?
$extra = !IS_WINDOWS ?
"unset REQUEST_METHOD; unset QUERY_STRING; unset PATH_TRANSLATED; unset SCRIPT_FILENAME; unset REQUEST_METHOD;" : "";
system_with_timeout("$extra $php $pass_options $extra_options -q $clean_params $no_file_cache \"$test_clean\"", $env);
}
Expand DownExpand Up@@ -2951,7 +2953,7 @@ function settings2params($ini_settings)
$settings .= " -d \"$name=$val\"";
}
} else {
if (substr(PHP_OS, 0, 3) == "WIN" && !empty($value) && $value[0] == '"') {
if (IS_WINDOWS && !empty($value) && $value[0] == '"') {
$len = strlen($value);

if ($value[$len - 1] == '"') {
Expand Down