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

phpunit6 - Implement TAP printer #14421

Merged
merged 2 commits into from
Jun 4, 2019
Merged

Conversation

seamuslee001
Copy link
Contributor

@seamuslee001 seamuslee001 commented Jun 3, 2019

Overview

This bridges a compatibility gap between PHPUnit5 and PHPUnit 6+ -- in that in the latter versions removed support for the --tap output format.

Before

  • tools/scripts/phpunit --tap works on PHP 5.x / PHPUnit 5
  • tools/scripts/phpunit --tap does not work on PHP 7.x / PHPUnit 6+

After

  • tools/scripts/phpunit --tap works on PHP 5.x / PHPUnit 5
  • tools/scripts/phpunit --tap works on PHP 7.x / PHPUnit 6+

Comments

This is a seamless update on any callers who use the tools/scripts/phpunit wrapper. However, if you call PHPUnit by other means, then you'll need to change --tap to --printer '\Civi\Test\TAP'.

ping @eileenmcnaughton

@civibot
Copy link

civibot bot commented Jun 3, 2019

(Standard links)

The `--tap` option was removed in PHPUnit 6.x.  For backwared compatibility,
the preceding commit adds a printer with same/similar output format.  This
continues adding the backward comapt by respecting `--tap` as an alias
for `--printer Civi/Test/TAP`.
@totten
Copy link
Member

totten commented Jun 3, 2019

Per civicrm/civicrm-buildkit#458 discussion, I've pushed up a revision so that tools/scripts/phpunit will continue respecting --tap (on older or newer versions of phpunit).

@totten totten changed the title Implement Civi Native TAP class phpunit - Implement TAP printer Jun 3, 2019
@totten
Copy link
Member

totten commented Jun 3, 2019

(Note: filled in a better description)

@totten totten changed the title phpunit - Implement TAP printer phpunit6 - Implement TAP printer Jun 3, 2019
@totten totten added the merge ready PR will be merged after a few days if there are no objections label Jun 4, 2019
@seamuslee001
Copy link
Contributor Author

Jenkins re test this please

1 similar comment
@seamuslee001
Copy link
Contributor Author

Jenkins re test this please

@totten
Copy link
Member

totten commented Jun 4, 2019

Test failures appear unrelated. (And the PR tests wouldn't really hit the new code path anyway.) The console output doesn't show anything weird.

On my local system, after checking both PHP 5.x and 7.x, the./tools/scripts/phpunit appears to enable (or not enable) the --tap option as expected.

@totten totten merged commit a988223 into civicrm:master Jun 4, 2019
@eileenmcnaughton eileenmcnaughton deleted the phpunit6_tap branch June 4, 2019 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master merge ready PR will be merged after a few days if there are no objections
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants