comparator icon indicating copy to clipboard operation
comparator copied to clipboard

Fix serialization of ComparisionFailure with PDO in stacktrace.

Open DomBlack opened this issue 7 years ago • 7 comments

This fix implements a custom serialize/deserialize method on the ComparisionFailure class, which means it loses the stack trace during serialization.

This allows PHPUnit to run in process isolation mode when the stack trace may contain references to non-serializable objects such as the PDO.

DomBlack avatar Mar 08 '18 12:03 DomBlack

Codecov Report

Merging #47 into master will increase coverage by 0.02%. The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #47      +/-   ##
============================================
+ Coverage     99.07%   99.09%   +0.02%     
- Complexity      123      125       +2     
============================================
  Files            15       15              
  Lines           325      333       +8     
============================================
+ Hits            322      330       +8     
  Misses            3        3
Impacted Files Coverage Δ Complexity Δ
src/ComparisonFailure.php 100% <100%> (ø) 11 <2> (+2) :arrow_up:

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 2256ef8...3fdfacd. Read the comment docs.

codecov-io avatar Sep 27 '18 09:09 codecov-io

@sebastianbergmann I've re based on the current master branch. Is there any chance of getting this merged?

Also I think the codecov-io report is for the previous version against the current master; rather than the current version of this PR

DomBlack avatar Sep 27 '18 09:09 DomBlack

@DomBlack The problem still exists so I wrote the patch: mpyw/phpunit-patch-serializable-comparison

mpyw avatar Jun 23 '21 16:06 mpyw

@sebastianbergmann @DomBlack We've been running into this issue in the Drupal community as well. I see this PR does need an update to use the __serialize and __unserialize magic methods introduced in php 7.4, rather than implementing the Serializable interface. If we can get an updated PR, I'd like to know the chances of being able to get it merged, as we could definitely use it downstream.

Related Drupal Core issue: https://www.drupal.org/project/drupal/issues/3197324

mdlutz24 avatar Jul 03 '22 13:07 mdlutz24

The only thing I can promise that I would review such a pull request.

sebastianbergmann avatar Jul 03 '22 17:07 sebastianbergmann

I am running into similar issue as well with having object as args on the stack trace that cannot be serialized when running in isolation.

Which is related to issue: sebastianbergmann/phpunit/issues/1351 and pull: sebastianbergmann/phpunit/issues/1352

In that PR, instead of trying to override serialization methods, the constructor just unset the args on each stack element: see commit for Framework/Exception.php

Relevant code from Framework/Exception.php in that PR:

    public function __construct($message = '', $code = 0, Exception $previous = null)
    {
        parent::__construct($message, $code, $previous);

        $this->serializableTrace = $this->getTrace();
        foreach ($this->serializableTrace as $i => $call) {
            unset($this->serializableTrace[$i]['args']);
        }
    }

maybe a similar approach could be utilized in ComparisonFailure?

-- edit/update to my own message... After digging into this one a bit more, seems that overriding the serialization methods probably is the best route, just leaving my original comment for additional information.

twoseascharlie avatar Jun 08 '23 16:06 twoseascharlie

Added a (very naive) first stab at updating the old PR in #106.

Boegie avatar Jul 01 '23 12:07 Boegie