[ Ready ] Upgrade (De)Serialization checks for PHP 7.4 #22
Conversation
|
Hi Ocramius. Should we continue the conversation in the issue or start anew here in the PR? I have a question about the red ❌ from travis. English is not m first language. What does it mean and what can I do to turn it into a green ✔️? |
Sorry for the stumbles. I usually get called out by the typechecker for mistakes like this.
lexidor
changed the title
|
I don't know what formatter you use. |
|
I would recommend a squashed merge, because there are many intermediary commits. |
| try { | ||
| $dont->__unserialize([]); | ||
| self::markAsFailed('This method must always throw'); | ||
| } catch (NonDeserialisableObject $_) { |
There was a problem hiding this comment.
This should be multiple test methods, using $this->expectException(NonDeserialisableObject::class) each
tests/DontTest/DontSerialiseTest.php
Outdated
|
|
||
| try { | ||
| $dont->__serialize(); | ||
| self::markAsFailed('This method must always throw'); |
There was a problem hiding this comment.
Similar to the above: need $this->expectException() and separate tests instead
| @@ -23,8 +24,11 @@ final class DontDeserialiseTest extends TestCase | |||
| public function testWillThrowOnSerialisationAttempt($className) : void | |||
| { | |||
| $this->expectException(NonDeserialisableObject::class); | |||
|
|
|||
| unserialize(\sprintf('O:%s:"%s":0:{}', \strlen($className), $className)); | |||
| if($className === 'DontTestAsset\\NonDeserialisableImplementingSerializable'){ | |||
There was a problem hiding this comment.
NonDeserialisableImplementingSerializable::class should be used here
Fixes #21