Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Replacing self:: with static:: in Http\Client::setAuth #6301

Closed

Conversation

tklever
Copy link
Contributor

@tklever tklever commented May 20, 2014

This becomes an issue if you want to create a custom authentication method in a class extending Http\Client

The self:: check only allows for the included constants (AUTH_BASIC and AUTH_DIGEST), and will throw an exception on AUTH_XXXX constants in extending classes.

@tklever
Copy link
Contributor Author

tklever commented May 20, 2014

I have a unit test for this if it is wanted. However, as there isn't any way to test for an exception NOT being thrown, I don't think it adds much that the current coverage on setAuth doesn't already provide.

@Martin-P
Copy link
Contributor

When you create a test asset it is possible to test if an exception is not thrown. Examples of this approach can be found in other parts: https://github.com/zendframework/zf2/tree/master/tests/ZendTest/Json/TestAsset

@Maks3w
Copy link
Member

Maks3w commented May 27, 2014

@tklever The most easy way for test an exception is

try {
  $method->call();
  $this->fail('Expected exception');
} catch (The\Expected\Exception $e) {
}

@DASPRiD
Copy link
Member

DASPRiD commented May 28, 2014

@Maks3w no, the easiest is the following ;)…

$this->setExpectedException('ExceptionName', 'String within exception message');

@tklever But as you said, you want to test that an exception is not thrown. Well, simply don't assert anything, the test will fail when an exception is thrown. Although in case you have no assertions, make a comment that it will fail based on an exception.

@tklever
Copy link
Contributor Author

tklever commented May 28, 2014

@DASPRiD, you just perfectly described the test I have. It asserts nothing. On paper it seemed kind of useless, so I didn't originally include it. I'll add some comments that explain its reason for existing and push them into this PR.

Ocramius added a commit that referenced this pull request Jul 28, 2014
Ocramius added a commit that referenced this pull request Jul 28, 2014
@Ocramius Ocramius self-assigned this Jul 28, 2014
@Ocramius Ocramius added this to the 2.4.0 milestone Jul 28, 2014
@Ocramius
Copy link
Member

@tklever manually rebased, merged and fixed the test so that there is an assertion. A bit risky, but I prefer it to adding a getter, which would be worse.

@Ocramius Ocramius closed this Jul 28, 2014
freax pushed a commit to freax/zf2 that referenced this pull request Nov 27, 2014
freax pushed a commit to freax/zf2 that referenced this pull request Nov 27, 2014
gianarb pushed a commit to zendframework/zend-http that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-http that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-http that referenced this pull request May 15, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants