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

Fix unit tests by aliasing PHPUnit_Framework_TestCase with namespaced name #280

Closed
wants to merge 15 commits into from

Conversation

TysonAndre
Copy link
Contributor

This change is intended to work in php 5.2+, the lowest version .travis.yml
indicates this supports.

If PHPUnit_Framework_TestCase does not exist, check if
PHPUnit\Framework\TestCase does exist, and if so, alias it.

  • Different versions of phpunit may be required for different php
    versions, so don't try standardizing for now.

Fixes #279

… name

This change is intended to work in php 5.2+, the lowest version .travis.yml
indicates this supports.

If PHPUnit_Framework_TestCase does not exist, check if
PHPUnit\Framework\TestCase does exist, and if so, alias it.

- Different versions of phpunit may be required for different php
  versions, so don't try standardizing for now.

Fixes WordPress#279
@TysonAndre
Copy link
Contributor Author

https://travis-ci.org/rmccue/Requests/jobs/239492433 failed for the below reason

https://phpunit.de/manual/current/en/writing-tests-for-phpunit.html

You should be as specific as possible when testing exceptions. Testing for classes that are too generic might lead to undesirable side-effects. Accordingly, testing for the Exception class with @ExpectedException or setExpectedException() is no longer permitted.

Will fix this later, or rebase if another PR fixes that.

@rmccue
Copy link
Collaborator

rmccue commented Jun 6, 2017

@TysonAndre The way I read that message is that when you call setExpectedException, you can't say you're expecting instances of Exception, but you can use more specific ones.

Based on sebastianbergmann/phpunit#2074 it seems like we just need to use $this->expectException instead.

Add setExpectedException shim for PHPUnit 6
@codecov-io
Copy link

codecov-io commented Jun 6, 2017

Codecov Report

Merging #280 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #280   +/-   ##
=======================================
  Coverage   92.11%   92.11%           
=======================================
  Files          21       21           
  Lines        1762     1762           
=======================================
  Hits         1623     1623           
  Misses        139      139

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 87932f5...1e4da14. Read the comment docs.

@rmccue
Copy link
Collaborator

rmccue commented Jun 9, 2017

🙌 Finally passing!

Needs a review from a committer though :)

@rmccue rmccue requested review from ozh, ptasker and robwilde June 9, 2017 05:34
$this->assertFalse(isset($iri->nonexistant_prop));
$should_fail = $iri->nonexistant_prop;
$this->assertFalse(isset($iri->nonexistent_prop));
$should_fail = $iri->nonexistent_prop;
Copy link
Collaborator

Choose a reason for hiding this comment

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

(These changes aren't relevant, I just happened to notice the misspelling as I was fixing it.)

@rmccue
Copy link
Collaborator

rmccue commented Jun 9, 2017

@TysonAndre Thanks for helping out with this issue (and on the others) by the way; you now have commit access. :)

@ptasker
Copy link
Collaborator

ptasker commented Jul 21, 2017

I've tried to get the tests running a few times and it keeps bailing on the fsockopen Transport tests. Any tips on getting those running?

PHPUnit 6.2.3
PHP 7.1.3

There were 3 failures:

1) RequestsTest_Transport_fsockopen::testResponseByteLimit
Failed asserting that 99 matches expected 104.

/Users/petertasker/working/Requests/tests/Transport/Base.php:48

2) RequestsTest_Transport_fsockopen::testStreamToFile
Failed asserting that null matches expected 'http://requests-php-tests.herokuapp.com/get'.

/Users/petertasker/working/Requests/tests/Transport/Base.php:489

3) RequestsTest_Transport_fsockopen::testMultipleToFile
Failed asserting that null matches expected 'http://requests-php-tests.herokuapp.com/get'.

/Users/petertasker/working/Requests/tests/Transport/Base.php:749

@ozh ozh mentioned this pull request Oct 23, 2017
The problem is now that phpenv fails on each job. Just testing to figure out why.
@ozh
Copy link
Collaborator

ozh commented Oct 24, 2017

Screw it. I'm done.

@ntwb ntwb mentioned this pull request Dec 12, 2017
@dd32 dd32 mentioned this pull request Dec 13, 2017
@rmccue
Copy link
Collaborator

rmccue commented Dec 13, 2017

Thanks for this @TysonAndre, we ended up fixing it finally in #303.

@rmccue rmccue closed this Dec 13, 2017
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.

5 participants