Skip to content

fix remaining skipped tests - #61

Merged
carlos-granados merged 2 commits into
php-debugger:mainfrom
carlos-granados:fix/skipped-tests
Apr 22, 2026
Merged

carlos-granados merged 2 commits into
php-debugger:mainfrom
carlos-granados:fix/skipped-tests

Conversation

@carlos-granados

Copy link
Copy Markdown
Collaborator

There were several tests which had been skipped as they failed in the new debugger. This PR fixes all the remaining cases. In most places we just need to adjust the test but there was an important bug where we had removed the call to the existing php error handler which resulted in several failures and which would have made error handling not work properly with the php-debugger installed.

This PR also removes all the tests which targeted versions of PHP lower than PHP 8.2

With these changes and the changes proposed in the two PRs which fix the remaining XFAIL tests, all tests are passing as in the initial fork 🎉

Comment thread src/base/base.c

xdfree(error_type_str);
}
xdebug_old_error_cb(orig_type, error_filename, error_lineno, message);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This was an important bug, we had removed calling the existing php error handler and this resulted in several issues

Comment thread tests/base/bug01048-002.phpt Outdated

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We removed all tests which targeted PHP versions lower than 8.2

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

After removing the tests that targeted versions lower than 8.2 we sometimes were left with versions of the tests which targeted all the versions that we handle. In that case it does not make sense to name them with a specific version, so I renamed them

<?php
require __DIR__ . '/../utils.inc';
check_reqs('dbgp');
if (is_stripped_debugger()) die('skip Known issue in stripped build');

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Bug fixed by the change in the error callback

var_dump( $_SERVER['I_LIKE_COOKIES'] );
?>
--EXPECTF--
Xdebug: [Step Debug] %sTried: 127.0.0.1:9999 (from I_LIKE_COOKIES HTTP header), localhost:9999 (fallback through xdebug.client_host/xdebug.client_port).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Our debugger does not print a message when it cannot connect, so this needs to be removed from the output of some tests

@@ -1,10 +1,5 @@
--TEST--
Test for bug #1901: Stack traces are shown (with a broken time) when Xdebug's mode includes 'debug' but not 'develop' or 'trace'
--SKIPIF--

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed by the change in the error callback

@@ -1,10 +1,5 @@
--TEST--
Test for bug #2250: Multiple 'DEBUG SESSION ENDED' messages
--SKIPIF--

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Not really sure which of our changes fixed this, but now it works

}
?>
--EXPECTF--
Xdebug: [Step Debug] %sdebugging client%s

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We don't print a message if we can't connect

@unlink(getTmpFile('bug02251.log'));
?>
--EXPECTF--
%A[Step Debug] %sTried: localhost:9172 (through xdebug.client_host/xdebug.client_port).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

What is logged is different as our start sequence is different, but this is not important for this test which actually tests that the log configuration is picked from the env variables

--ENV--
XDEBUG_CONFIG=log={TMPFILE:bug02251.log}
--INI--
xdebug.mode=debug,develop

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is not important for the test

<?php
require __DIR__ . '/../utils.inc';
check_reqs('dbgp');
if (is_stripped_debugger()) die('skip Known issue in stripped build');

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed by our change in the error handler

[%d] [Step Debug] WARN: Invalid remote address provided containing URI spec 'unix:///tmp/haxx0r.sock'.
[%d] [Step Debug] WARN: Could not discover client host through HTTP headers, connecting to configured address/port: unix:///tmp/xdbg.sock:0.
[%d] [Step Debug] WARN: Creating socket for 'unix:///tmp/xdbg.sock', connect: No such file or directory.
[%d] [Step Debug] ERR: Could not connect to debugging client. Tried: unix:///tmp/xdbg.sock:0 (fallback through xdebug.client_host/xdebug.client_port).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

In the logs we don't log an error when we can't connect to a client

'step_into',
'context_get',
'property_get -n $__RETURN_VALUE',
'property_get -n $__RETURN_VALUE[1]',

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The stack that we get does not have as much detail as the one provided by xdebug. However this is not very important for this test which actually tests that we can access the return value when the function at the top of the frame is internal, so checking for less detail is fine

# Conflicts:
#	src/base/base.c
Comment thread src/debugger/com.c
xdfree(header_name);
}

xdebug_arg_dtor(headers);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Includes some fixes to memory leaks found by USBAN. These had not been seen before because the tests that run this code were skipped

@pronskiy pronskiy left a comment

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.

👍

@carlos-granados
carlos-granados merged commit b209bc5 into php-debugger:main Apr 22, 2026
14 checks passed
@carlos-granados
carlos-granados deleted the fix/skipped-tests branch April 22, 2026 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants