case-insensitive for Operator name#93
Conversation
| ->isTrue(); | ||
| } | ||
|
|
||
| public function case_insensitive() |
There was a problem hiding this comment.
I am pretty sure we can write unit test cases here. Thoughts?
There was a problem hiding this comment.
The reason is that we do not require an integration test case to check that Asserter::$_operators has change operator's name cases. This is a typical case of a unit test.
There was a problem hiding this comment.
It's true, but this case also cover the visitor rule are as expected, not only how operator are stored.
You think these three test are bit overkill?
There was a problem hiding this comment.
You only need to test the asserter visitor. Why do you want to test something else?
|
@Hywan Unit Test like this ? |
| ) | ||
| ->when($result = $asserter->setOperator('_FOO_', $operator)) | ||
| ->then | ||
| ->boolean($asserter->operatorExists('_foo_')) |
There was a problem hiding this comment.
You can also check that _FOO_ does not exist.
|
@Pierozi It looks good modulo my comment 👍. |
Pierozi
force-pushed
the
fix/setOperatorInsensitive
branch
from
9f10a86 to
e6bea82
Compare
|
Done 👍 |
|
I am assigning @jubianchi for the review! |
|
Quick review ? |
Hywan
left a comment
Hywan
left a comment
There was a problem hiding this comment.
The getter should ony change the case.
Also, in the Operator constructor, we must change the case there, instead of when instanciating the Operator class.
| { | ||
| $this | ||
| ->given( | ||
| $asserter = new SUT(), |
There was a problem hiding this comment.
Please, run hoa devtools:cs, and simply align =.
| ->isEqualTo(xcallable($operator)); | ||
| } | ||
|
|
||
| public function case_set_operator_insensitive() |
Unit test operator case-insensitive Complete Unit for operator case-insensitive
Pierozi
force-pushed
the
fix/setOperatorInsensitive
branch
from
4ca678a to
388e75e
Compare
2 similar comments
|
It seems casting of Extended ASCII work only from php >= 5.6 as Travis build report. |
|
@Pierozi In this case, I propose to wait on PHP 7 to land in Hoa to merge this PR. It will come really soon. Agree? |
4 participants
Address to #92Fix #92.
Edit by @Hywan.