case-insensitive for Operator name #93

Pierozi · 2016-09-14T09:36:20Z

~~Address to #92~~
Fix #92.

Edit by @Hywan.

Hywan · 2016-09-14T09:40:08Z

Test/Integration/Model/Operator.php

@@ -204,4 +204,30 @@ function ($b) use (&$gExecuted) {
                ->boolean($gExecuted)
                    ->isTrue();
    }
+
+    public function case_insensitive()


I am pretty sure we can write unit test cases here. Thoughts?

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.

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?

You only need to test the asserter visitor. Why do you want to test something else?

Pierozi · 2016-09-14T12:31:16Z

@Hywan Unit Test like this ?
I will rebase when it's ok.

Hywan · 2016-09-14T12:54:04Z

Test/Unit/Visitor/Asserter.php

+            )
+            ->when($result = $asserter->setOperator('_FOO_', $operator))
+            ->then
+                ->boolean($asserter->operatorExists('_foo_'))


You can also check that _FOO_ does not exist.

Hywan · 2016-09-14T12:54:18Z

@Pierozi It looks good modulo my comment 👍.

Pierozi · 2016-09-14T13:03:23Z

Done 👍

Hywan · 2016-09-14T13:26:34Z

I am assigning @jubianchi for the review!

Pierozi · 2017-01-11T00:38:31Z

Quick review ?

Hywan

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.

Hywan · 2017-01-11T07:45:53Z

Test/Unit/Visitor/Asserter.php

+    {
+        $this
+            ->given(
+                $asserter     = new SUT(),


Please, run hoa devtools:cs, and simply align =.

Hywan · 2017-01-11T07:48:59Z

Test/Unit/Visitor/Asserter.php

@@ -418,6 +418,23 @@ public function case_set_operator()
                    ->isEqualTo(xcallable($operator));
    }

+    public function case_set_operator_insensitive()


Please, add a case with a non-ASCII symbol, like Λ and λ (resp the capital and small variant). Is it what we want? I guess yes.

Unit test operator case-insensitive Complete Unit for operator case-insensitive

coveralls · 2017-03-16T23:10:23Z

Coverage increased (+0.2%) to 96.132% when pulling 388e75e on Pierozi:fix/setOperatorInsensitive into c9a965c on hoaproject:master.

coveralls · 2017-03-16T23:10:23Z

Coverage increased (+0.2%) to 96.132% when pulling 388e75e on Pierozi:fix/setOperatorInsensitive into c9a965c on hoaproject:master.

coveralls · 2017-03-16T23:10:23Z

Coverage increased (+0.2%) to 96.132% when pulling 388e75e on Pierozi:fix/setOperatorInsensitive into c9a965c on hoaproject:master.

Pierozi · 2017-03-16T23:28:08Z

It seems casting of Extended ASCII work only from php >= 5.6 as Travis build report.

Hywan · 2017-04-06T11:48:16Z

@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?

Hywan added in progress enhancement difficulty: medium labels Sep 14, 2016

Hywan self-assigned this Sep 14, 2016

Hywan reviewed Sep 14, 2016
View reviewed changes

Pierozi force-pushed the fix/setOperatorInsensitive branch from 9f10a86 to e6bea82 Compare September 14, 2016 13:02

Hywan assigned jubianchi and unassigned Hywan Sep 14, 2016

Hywan suggested changes Jan 11, 2017

View reviewed changes

Pierozi assigned Pierozi and unassigned jubianchi Mar 16, 2017

Pierozi requested a review from jubianchi March 16, 2017 23:01

Pierozi added 4 commits March 17, 2017 00:04

cast operator name for insensitive compatibility

300e30e

Unit test operator case-insensitive Complete Unit for operator case-insensitive

feat(case-insensitive operator) handle cast to lower on public methods

d4235a6

feat(case-insensitive operator) test cast to lower with non-ASCII

62e5f73

CS

388e75e

Pierozi force-pushed the fix/setOperatorInsensitive branch from 4ca678a to 388e75e Compare March 16, 2017 23:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

case-insensitive for Operator name #93

case-insensitive for Operator name #93

Pierozi commented Sep 14, 2016 •

edited by Hywan

Hywan Sep 14, 2016

Hywan Sep 14, 2016

Pierozi Sep 14, 2016

Hywan Sep 14, 2016

Pierozi commented Sep 14, 2016

Hywan Sep 14, 2016

Hywan commented Sep 14, 2016 •

edited

Pierozi commented Sep 14, 2016

Hywan commented Sep 14, 2016

Pierozi commented Jan 11, 2017

Hywan left a comment

Hywan Jan 11, 2017

Hywan Jan 11, 2017

coveralls commented Mar 16, 2017

coveralls commented Mar 16, 2017

coveralls commented Mar 16, 2017 •

edited

Pierozi commented Mar 16, 2017

Hywan commented Apr 6, 2017

case-insensitive for Operator name #93

Are you sure you want to change the base?

case-insensitive for Operator name #93

Conversation

Pierozi commented Sep 14, 2016 • edited by Hywan

Hywan Sep 14, 2016

Choose a reason for hiding this comment

Hywan Sep 14, 2016

Choose a reason for hiding this comment

Pierozi Sep 14, 2016

Choose a reason for hiding this comment

Hywan Sep 14, 2016

Choose a reason for hiding this comment

Pierozi commented Sep 14, 2016

Hywan Sep 14, 2016

Choose a reason for hiding this comment

Hywan commented Sep 14, 2016 • edited

Pierozi commented Sep 14, 2016

Hywan commented Sep 14, 2016

Pierozi commented Jan 11, 2017

Hywan left a comment

Choose a reason for hiding this comment

Hywan Jan 11, 2017

Choose a reason for hiding this comment

Hywan Jan 11, 2017

Choose a reason for hiding this comment

coveralls commented Mar 16, 2017

coveralls commented Mar 16, 2017

coveralls commented Mar 16, 2017 • edited

Pierozi commented Mar 16, 2017

Hywan commented Apr 6, 2017

Pierozi commented Sep 14, 2016 •

edited by Hywan

Hywan commented Sep 14, 2016 •

edited

coveralls commented Mar 16, 2017 •

edited