Skip to content

Tests: Improve Windows compatibility #200

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

Merged
swissspidy merged 10 commits into from
Mar 29, 2026
+25 −15
Merged

Tests: Improve Windows compatibility #200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
625f989
Normalize line endings in `assertOutFileEqualsWith`
swissspidy Mar 29, 2026
a4e18af
Check `COLUMNS` early and on all platforms
swissspidy Mar 29, 2026
a1efaef
Improve tests cleanup
swissspidy Mar 29, 2026
4e8771a
Add Windows specific assertion
swissspidy Mar 29, 2026
34178c6
try fix
swissspidy Mar 29, 2026
e53d0a1
another one
swissspidy Mar 29, 2026
aee0944
another one
swissspidy Mar 29, 2026
de381af
another one
swissspidy Mar 29, 2026
c9ccc1a
merge conditions
swissspidy Mar 29, 2026
a0cd69a
fix \r
swissspidy Mar 29, 2026
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: 9 additions & 11 deletions lib/cli/Shell.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ static public function columns() {
$columns = null;
}
if ( null === $columns ) {
if ( function_exists( 'exec' ) ) {
if ( ! ( $columns = (int) getenv( 'COLUMNS' ) ) && function_exists( 'exec' ) ) {
if ( self::is_windows() ) {
// Cater for shells such as Cygwin and Git bash where `mode CON` returns an incorrect value for columns.
if ( ( $shell = getenv( 'SHELL' ) ) && preg_match( '/(?:bash|zsh)(?:\.exe)?$/', $shell ) && getenv( 'TERM' ) ) {
Expand All @@ -49,15 +49,13 @@ static public function columns() {
}
}
} else {
if ( ! ( $columns = (int) getenv( 'COLUMNS' ) ) ) {
$size = exec( '/usr/bin/env stty size 2>/dev/null' );
if ( '' !== $size && preg_match( '/[0-9]+ ([0-9]+)/', $size, $matches ) ) {
$columns = (int) $matches[1];
}
if ( ! $columns ) {
if ( getenv( 'TERM' ) ) {
$columns = (int) exec( '/usr/bin/env tput cols 2>/dev/null' );
}
$size = exec( '/usr/bin/env stty size 2>/dev/null' );
if ( '' !== $size && preg_match( '/[0-9]+ ([0-9]+)/', $size, $matches ) ) {
$columns = (int) $matches[1];
}
if ( ! $columns ) {
if ( getenv( 'TERM' ) ) {
$columns = (int) exec( '/usr/bin/env tput cols 2>/dev/null' );
}
}
}
Expand Down Expand Up @@ -114,7 +112,7 @@ static public function hide($hidden = true) {
*/
static public function is_windows() {
$test_is_windows = getenv( 'WP_CLI_TEST_IS_WINDOWS' );
if ( false !== $test_is_windows ) {
if ( false !== $test_is_windows && '' !== $test_is_windows ) {
return (bool) $test_is_windows;
}
return strtoupper( substr( PHP_OS, 0, 3 ) ) === 'WIN';
Expand Down
10 changes: 9 additions & 1 deletion tests/Test_Shell.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,15 @@ function testColumns() {
// Restore.
putenv( false === $env_term ? 'TERM' : "TERM=$env_term" );
putenv( false === $env_columns ? 'COLUMNS' : "COLUMNS=$env_columns" );
putenv( false === $env_is_windows ? 'WP_CLI_TEST_IS_WINDOWS' : "WP_CLI_TEST_IS_WINDOWS=$env_is_windows" );
if ( false === $env_is_windows ) {
if ( strtoupper( substr( PHP_OS, 0, 3 ) ) === 'WIN' ) {
putenv( 'WP_CLI_TEST_IS_WINDOWS=' );
} else {
putenv( 'WP_CLI_TEST_IS_WINDOWS' );
}
} else {
putenv( "WP_CLI_TEST_IS_WINDOWS=$env_is_windows" );
}
Comment on lines +52 to +60
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 29, 2026

Choose a reason for hiding this comment

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

high

This change to restore the WP_CLI_TEST_IS_WINDOWS environment variable introduces a problem for tests running on Windows.

On Windows, putenv('WP_CLI_TEST_IS_WINDOWS=') sets the variable to an empty string. Consequently, is_windows() will return false because (bool)'' is false. This breaks test isolation, as subsequent tests will incorrectly detect the OS as non-Windows.

The underlying issue is in is_windows(), which doesn't differentiate an unset variable from one set to an empty string. The ideal fix is to make is_windows() more robust, for instance:

if (false !== $test_is_windows && '' !== $test_is_windows) {
    return (bool) $test_is_windows;
}

Since modifying is_windows() might be out of scope for this file, this restoration logic needs to be re-evaluated to avoid breaking other tests.

putenv( false === $env_shell_columns_reset ? 'PHP_CLI_TOOLS_TEST_SHELL_COLUMNS_RESET' : "PHP_CLI_TOOLS_TEST_SHELL_COLUMNS_RESET=$env_shell_columns_reset" );
}
}
5 changes: 3 additions & 2 deletions tests/Test_Table.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use cli\Colors;
use cli\Table;
use cli\Table\Ascii;
use cli\Shell;
use WP_CLI\Tests\TestCase;

/**
Expand Down Expand Up @@ -85,7 +86,7 @@ public function test_column_odd_single_width_with_double_width() {

$strip_borders = function ( $a ) {
return array_map( function ( $v ) {
return substr( $v, 2, -2 );
return substr( rtrim( $v, "\r" ), 2, -2 );
}, $a );
};

Expand All @@ -96,7 +97,7 @@ public function test_column_odd_single_width_with_double_width() {
$result = $strip_borders( explode( "\n", $out ) );

$this->assertSame( 3, count( $result ) );
$this->assertSame( '1あいうえ ', $result[0] ); // 1 single width, 4 double-width, space = 10.
$this->assertSame( '1あいうえ ', $result[0] ); // 1 single-width, 4 double-width, space = 10.
$this->assertSame( 'おか2きくｶ', $result[1] ); // 2 double-width, 1 single-width, 2 double-width, 1 half-width = 10.
$this->assertSame( 'けこ ', $result[2] ); // 2 double-width, 8 spaces = 10.

Expand Down
5 changes: 4 additions & 1 deletion tests/Test_Table_Ascii.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,9 @@ private function assertInOutEquals(array $input, $output) {
* @param mixed $expected Expected output
*/
private function assertOutFileEqualsWith($expected) {
$this->assertEquals($expected, file_get_contents($this->_mockFile));
$actual = file_get_contents($this->_mockFile);
$actual = str_replace("\r\n", "\n", $actual);
$expected = str_replace("\r\n", "\n", $expected);
$this->assertEquals($expected, $actual);
}
}