jubianchi / atoum

The modern, simple and intuitive PHP 5.3+ unit testing framework.
Other
1 stars 1 forks source link

Strange bug with instrumentation #5

Open jubianchi opened 10 years ago

jubianchi commented 10 years ago

Using the following config. file and the commands : https://gist.github.com/jubianchi/da7ba5b4d81a489b793f

We get a strange error when running tests :

classes/instrumentation/coverage.php on line 26:
Access to undeclared static property: mageekguy\atoum\instrumentation\coverage::$_scores

Things to know:

Any clue ?

/cc @Hywan

Hywan commented 10 years ago

Hello :-),

So far, I have found two solutions to fix the bug. Either adding isset(static::$_scores) in coverage::mark & co., or remove the cache in stream\controller::stream_open. I continue to investigate.

Hywan commented 10 years ago

Ok, the investigation goes on. When the files

classes/mock/streams/fs/file/controller.php",
classes/mock/stream.php",
classes/test/adapter/call.php",
classes/test/adapter/call/decorator.php",
classes/test/adapter/calls.php",

are instrumented, we got an error. But, I try to run all the tests, and we have new files that appear as “bug-generator”. I have no clue… seems like it is a bug from PHP I can't isolate…

Hywan commented 10 years ago

I'm not sure that the error comes from the instrumentation :-/. I wonder if it does not come from the concurrent execution or something like that, especially because a stupid isset solve the issue (it will tell PHP to update an internal hashtable or something else…).

Hywan commented 10 years ago

I tried with the inline engine, same issue. But I tried without the cache and the bug disappeared. The cache seems to be the origin of the issue.

Hywan commented 10 years ago

Nop. I find a test that shits bricks. Forget my comment.

Hywan commented 10 years ago

When I run one single test (for example instrumentation\stream\controller::testStream_read), no problem. When I run several test (still from instrumentation\stream\controller), I have the issue.

Hywan commented 10 years ago

Ok, I progress. Try to run:

$ bin/atoum -d tests/units/classes/instrumentation/stream/ -m 'mageekguy\atoum\tests\units\instrumentation\stream\controller::testStream_read' ++verbose

With the defaultEngine set to inline, we have no bug, while set to isolate produce an error. The only difference is the fork no? (with the instrumentation cache disabled)

Hywan commented 10 years ago

With cache enabled and defaultEngine set to inline, we still have the bug.

Hywan commented 10 years ago

Ok. My patch fix everything and I don't know why. Here it is:

diff --git a/classes/instrumentation/coverage.php b/classes/instrumentation/coverage.php
index 9d57f3c..ff35ac3 100644
--- a/classes/instrumentation/coverage.php
+++ b/classes/instrumentation/coverage.php
@@ -23,13 +23,16 @@ class coverage
     }

     public static function mark ( $id, $index ) {
-        if(isset(static::$_scores[$id]))
+
+        if(isset(static::$_scores) && isset(static::$_scores[$id]))
             static::$_scores[$id][$index] = true;
+
+        return;
     }

     public static function markCondition ( $id, $index, $condition ) {

-        if(isset(static::$_scores[$id]))
+        if(isset(static::$_scores) && isset(static::$_scores[$id]))
             static::$_scores[$id][$index] = static::$_scores[$id][$index] || true == $condition;

         return $condition;
diff --git a/classes/instrumentation/stream/controller.php b/classes/instrumentation/stream/controller.php
index 4207793..1e724f2 100644
--- a/classes/instrumentation/stream/controller.php
+++ b/classes/instrumentation/stream/controller.php
@@ -91,7 +91,7 @@ class controller
                 $cache->lock();
             }

-            $stream= $this->adapter->fopen($path, $mode, $options & STREAM_USE_PATH);
+            $stream = $this->adapter->fopen($path, $mode, $options & STREAM_USE_PATH);

             if ($this->adapter->is_resource($stream))
             {
jubianchi commented 10 years ago

I think I got the error : it's because of the missing space before the = here $stream = $this->adapter->fopen($path, $mode, $options & STREAM_USE_PATH);

:laughing:

Thanks for your work on this, I'll apply the patch asap and let you know :)

Hywan commented 10 years ago

Ok thanks :-).