Skip to content

Add support for pcov #137

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

Merged
merged 1 commit into from
Sep 24, 2021
Merged

Add support for pcov #137

merged 1 commit into from
Sep 24, 2021

Conversation

andrewnicols
Copy link
Contributor

Add support for pcov such that we may generate code coverage with relative ease.

@andrewnicols andrewnicols force-pushed the pcov branch 5 times, most recently from c20c46f to a372bff Compare September 24, 2021 13:55
@stronk7
Copy link
Member

stronk7 commented Sep 24, 2021

Hi,

as agreed @ chat... I'm going to pull the initial iteration of this into master, do some tests and spread it over >= 7.3 images.

So, for now we can enable pcov by doing simply:

docker run moodlehq/moodle-php-apache  php -dpcov.enabled=1 vendor/bin/phpunit

Pending is the idea about how we could pass any arbitrary number of INI settings to the images, surely using some env variable containing all them and then an entry point adding them to ini file. Something like that would also help to issues/prs like #107.

Ok, going to try...

@stronk7 stronk7 merged commit 9bc9a25 into moodlehq:master Sep 24, 2021
@stronk7
Copy link
Member

stronk7 commented Sep 24, 2021

Perfect, have run:

$ docker exec bc7ef826d47c php -dpcov.enabled=1 vendor/bin/phpunit --coverage-html cc  \
    --filter dml_test --coverage-filter lib/dml
Moodle 4.0dev (Build: 20210924), 214adb798498741f9ddfe3533efcb106db8ad6ec
Php: 8.0.11, pgsql: 13.3, OS: Linux 5.10.47-linuxkit x86_64
PHPUnit 9.5.4 by Sebastian Bergmann and contributors.

...............................................................  63 / 109 ( 57%)
..............................................                  109 / 109 (100%)

Time: 00:08.786, Memory: 581.00 MB

OK (109 tests, 1245 assertions)

Generating code coverage report in HTML format ... done [00:49.712]

And have got coverage for lib/dml pretty nice generated:

ccpcov

@stronk7
Copy link
Member

stronk7 commented Sep 24, 2021

So I'm going to spread the commit over the 7.3, 7.4 and 8.0 branches to get all them able to run pcov coverage with phpunit.

@scara
Copy link
Contributor

scara commented Sep 25, 2021

Hello Everyone,

Something like that would also help to issues/prs like #107.

I still like the approach of a372bff#diff-d0f2543d6dfb906e651ff5d4cc4cb666134f65b3ddd1400ca430452e0ef7466a which could be evolved into using ENV VARs with the prefix PHP_INI_[EXT_<estension name>_], being automagically "upserted" by means of a custom entrypoint, both into each extension related ini file - some/each module potentially has its own ini file - and/or something like /usr/local/etc/php/conf.d/zzz-docker-php-from-env-vars.ini to set directives at the very end of the include files process forcing PHP to use the last parsed directive - not to mention that php.ini itself supports ENV VARs too on its own when using our own ini files and we could add our own supported PHP_INI_* with default values. based on the requests until today:

; PHP_INI_MEMORY_LIMIT is taken from environment
memory_limit = ${PHP_INI_MEMORY_LIMIT}

On the other side, @stronk7's proposal recalls me something like PHP_OPTS - to mimic JAVA_OPTS 😉 - to collect there a list of -dfoo[=bar] being applied by the custom entrypoint which could become an alias - with high precedence in PATH - of the actual php binary - on-demand -dfoo[=bar] approach is still a valid option for some use cases like this one about pcov.

Probably, a combination of the approaches above could be the long-term solution. Thought?

HTH,
Matteo

@stronk7
Copy link
Member

stronk7 commented Sep 25, 2021

Hi,

IMO, problem with supporting "custom" env variables is that there are endless combinations. The proposal here was about php-pcov ones, there are other issues about php-xdebug ones (#31, #102). Or there are ideas about how to edit php.ini settings (#107). Tomorrow it can be something related to the php-xxxx extension, or maybe max_execution_time or anything else.

So I was thinking that creating "custom" env variables could become a nightmare and won't solve all potential cases.

So (super draft explanations come), yes, I'was thinking about only ONE way to pass ANY php.ini setting and make the container to apply for them before starting. Say:

... -dPHP_memorysize=256M -d PHP_pcov.enabled=1 ...

Or, all-together:

-dPHP_INI="memorysize=256M;pcov.enabled=1;..."

That could be used for the php-pcov ones, for the php-xdebug ones, and for any other extension or setting in general. All them added to a final "zzzz" ini file, agree. And done.

(as commented above, yes, this is a very simple draft, all those settings can be passed in a number of ways (format-wise), but the idea remains).

Maybe we can create an issue with a proposal and continue advancing there. Once having it... I think it can also help to #140 (apart from the issues linked above) by having a clear and simple way to add any configuration to the image before it runs.

Ciao :-)

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.

3 participants