Skip to content
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.

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

Code coverage and phpunit: xdebug extension missing #17

Closed
scara opened this issue Nov 8, 2017 · 20 comments
Closed

Code coverage and phpunit: xdebug extension missing #17

scara opened this issue Nov 8, 2017 · 20 comments

Comments

@scara
Copy link
Contributor

scara commented Nov 8, 2017

Today, while scripting a test case in sebastianbergmann/phpunit#2844 (comment), I realized that here the xdebug extension is missing.

This extension could slow down the moodle docker environment so it should be provided disabled by default and enabled only when required e.g. for the phpunit code coverage.

@danpoltawski
Copy link
Contributor

+1 to the suggestion and way to implement it

@andrewnicols
Copy link
Contributor

+1 here too.

@caperneoignis
Copy link

I actually made these as too separate images, one with xdebug and one with out for every php version. I added a ARG to the docker file and in the travis test I add an additional env variable to the build, then tag the docker image with '_xdebug'. So php7.2 or 7.1 would be the normal files, and then if you want xdebug it would be 7.2_xdebug etc.

@caperneoignis
Copy link

I also add an additional item in the test file to test for xdebug if the env value is set during the test process.

@scara
Copy link
Contributor Author

scara commented Mar 22, 2018

Hi @caperneoignis,
would you like to contribute here an enhanced version w/ xdebug compiled but disabled and enabled only via an env var passed to the image?
We cannot double the images, for maintenance purposes.

Otherwise, in my spare time I'll create a PR.
Let me (us) know.

TIA,
Matteo

@caperneoignis
Copy link

Oh, I forgot you'll don't use travis to push the images. Because the tagging is handled by travis in my case. Let me take a look because disabling/enabling it on the fly when the image is started is tricky without a pre-built php.ini file to load with a string that can be replaced by a SED function. That's why I just have two images that are built by travis and have travis tag them in the job.

@scara
Copy link
Contributor Author

scara commented Nov 2, 2019

Hello Everyone,
a new PR landed to address this issue, #102, which requires re-building the image.

I'm still voting #31, but we should take into account non-root usage, #1, which could prevent to write the xdebug ini file where it should be so #102 could be an interesting option.

HTH,
Matteo

@andrewnicols
Copy link
Contributor

I'm voting against this. There is no need to use xdebug for code coverage when there are better solutions available which do not require the installation of extensions.

@caperneoignis
Copy link

@andrewnicols generally curious, what solution are you referring too that can be added on the fly and will work in the same fashion as xdebug, using the same files that Moodle outputs during test setup, and similar output? Or is the solution you are referring to take additional steps to setup?

@rezaies
Copy link

rezaies commented Dec 23, 2019

+1

@andrewnicols
Copy link
Contributor

andrewnicols commented Dec 23, 2019 via email

@scara
Copy link
Contributor Author

scara commented Dec 24, 2019

Hi @andrewnicols,
for the purpose of this issue I'm fine w/ your last sentence since a comment from @kabalin in moodlehq/moodle-docker#111 (comment).
Not sure why I was not able to collect coverage at the time of opening this issue though.

Since that time someone developing on top of Moodle HQ Docker Toolbox is asking for xdebug for more than the purpose of this issue.

Here is my proposal:

  1. [me] try to recap why I was not able to performe the coverage
  2. document it
  3. document how to extend the image

Both (2) and (3) in the README via PR.

HTH,
Matteo

@andrewnicols
Copy link
Contributor

andrewnicols commented Dec 24, 2019 via email

@scara
Copy link
Contributor Author

scara commented Jan 30, 2020

[OT] For my record, a note about using phpdbg for the code coverage: https://github.com/krakjoe/pcov#differences-in-reporting. Nothing really important for the purpose of this issue.

@abias
Copy link
Contributor

abias commented Feb 4, 2020

While this issue here is mostly about using XDebug for code coverage, it seems also to be generally about adding XDebug to the webserver container and to use it for things like live debugging.

We have been using a fork of moodle-php-apache which contained the additions of #31 for a long time now. However, we wanted to get rid of this fork and wanted to keep using XDebug for live debugging at the same time.

I have now a local management bash script which will add XDebug to a running webserver container:

moodle-docker-compose exec webserver pecl install xdebug
moodle-docker-compose exec webserver docker-php-ext-enable xdebug
read -r -d '' conf <<'EOF'
; Settings for Xdebug Docker configuration
xdebug.coverage_enable = 0
xdebug.default_enable = 0
xdebug.cli_color = 2
xdebug.file_link_format = phpstorm://open?%f:%l
xdebug.idekey = PHPSTORM
xdebug.remote_enable = 1
xdebug.remote_autostart = 1
xdebug.remote_host = host.docker.internal
EOF
moodle-docker-compose  exec webserver bash -c "echo '$conf' >> /usr/local/etc/php/conf.d/docker-php-ext-xdebug.ini"

The script is also able to disable XDebug on the fly:

moodle-docker-compose  exec webserver sed -i 's/^zend_extension=/; zend_extension=/' /usr/local/etc/php/conf.d/docker-php-ext-xdebug.ini
moodle-docker-compose restart

and to enable it again on the fly:

moodle-docker-compose  exec webserver sed -i 's/^; zend_extension=/zend_extension=/' /usr/local/etc/php/conf.d/docker-php-ext-xdebug.ini
moodle-docker-compose restart

I see that there are valid reasons not to add XDebug to the webserver container by default.
With the given commands, this is not necessary anymore for us.

Cheers,
Alex

@scara
Copy link
Contributor Author

scara commented Feb 4, 2020

At the end, thanks to @kabalin via moodlehq/moodle-docker@46cccbe and to @abias via #17 (comment) I'm fine w/ closing this issue.

Many thanks to @micaherne for #31 (at run time) and to @alikarim (at build time) for #102 for their proposals, too.

Let @andrewnicols and @stronk7 have the last word.

@stronk7
Copy link
Member

stronk7 commented Jun 18, 2020

I'm lost with so many issues about xdebug! :-D

@stronk7
Copy link
Member

stronk7 commented Sep 25, 2021

Hi,

just noting that the current images (php 7.3 and up) now come with php_pcov installed, and disabled by default so anybody using xdebug (for code coverage, debugging, ....) or other solutions should continue working without modifications. Reference: #137

And, if you want to run code coverage for the phpunit runs all you've to do is to change the command from:

docker exec <<container>>  vendor/bin/phpunit ...

to:

docker exec <<container>> php -dpcov.enabled=1 vendor/bin/phpunit ...

Ciao :-)

PS: I think I'm going to close this, as far as code-coverage and phpunit are now "covered". Will wait some time, to allow everybody to object.

@scara
Copy link
Contributor Author

scara commented Sep 25, 2021

Hi @stronk7,
+1 to close this issue which tells about code coverage via xdebug as, at that time, the supposed unique option.
We now have at least two options that should be mentioned into the README:

xdebug could be still a requirement for those who want it when developing and #17 (comment) by @abias should be documented someway into README too, for every developer convenience.

HTH,
Matteo

@stronk7
Copy link
Member

stronk7 commented Sep 25, 2021

I've created #140 about adding the phpdbg / pcov / xdebug details to README, for sure that will help people to find the information easier, agree.

@stronk7 stronk7 closed this as completed Sep 25, 2021
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

No branches or pull requests

7 participants