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

dev/core#980 [phpunit test] update phpunit extends class to support later versions #14333

Merged
merged 1 commit into from
May 30, 2019

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Updates phpunit version support in the test suite

Before

Supported versions of phpunit (7 & 8) do not work with CiviCRM test suite.

After

Supported versions of phpunit (7 & 8) do work with CiviCRM test suite. Versions prior to 4.8 do not

Technical Details

Per https://engineering.facile.it/blog/eng/phpunit-upgrade-namespace/ this works with phpunit 4.8 +

Phpunit support for php versions is here https://phpunit.de/supported-versions.html

Basically we need phpunit 7 to support php 7.3 - which we should be adding to our
test suite now. It's hard to make a case that we still need to support phpunit 4.8 - let
alone versions before that - 5.7 still covers php 5.6 which we need for some of our current branches.

Unit testing on master would ideally switch up to a supported version of phpunit - 7 or 8

Comments

@totten @seamuslee001

Per https://engineering.facile.it/blog/eng/phpunit-upgrade-namespace/ this works with phpunit 4.8 +

Phpunit support for php versions is here https://phpunit.de/supported-versions.html

Basically we need phpunit 7 to support php 7.3 - which we should be adding to our
test suite now. It's hard to make a case that we still need to support phpunit 4.8 - let
alone versions before that - 5.7 still covers php 5.6 which we need for some of our current branches.

Unit testing on master would ideally switch up to a supported version of phpunit - 7 or 8
@civibot
Copy link

civibot bot commented May 25, 2019

(Standard links)

@civibot civibot bot added the master label May 25, 2019
@eileenmcnaughton
Copy link
Contributor Author

test this please

@seamuslee001
Copy link
Contributor

Given this only affects tests and Jenkins is ok with it then i think we should merge it, Jenkins should be testing primarily using phpunit5. Given that min is 5.6 it should be caught by this code

if (version_compare(PHP_VERSION, '5.6', '>=')) {
  $phpunit = findCommand('phpunit5');
}

that is in tools/scripts/phpunit

@seamuslee001 seamuslee001 merged commit bbccc5c into civicrm:master May 30, 2019
@seamuslee001 seamuslee001 deleted the phpunit branch May 30, 2019 22:17
@eileenmcnaughton
Copy link
Contributor Author

woot

@totten
Copy link
Member

totten commented May 31, 2019

Sweet. Agree this is a good idea for forward compatibility. And in this case... if it passes (esp on bknix-{min,max,dfl}), then it passes. :)

@seamuslee001
Copy link
Contributor

@totten we are hitting some issues with further changes that are needed for a PHPUnit6 test runner see #14387

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants