Open meeuw opened 2 years ago
I've been pondering this tonight while source diving. I've been trying to understand what role the driver and filter have once the data has been collected and is just being serialized. Forgive my ignorance if I'm asking questions that don't make sense or have obvious answers.
Is the intention here to have a new JSON format, specific to this package which contains the data needed to instantiate a CodeCoverage
instance? And thereby to be able to that JSON format and unserialize from that format?
Can we define the JSON schema such that it has the data we need? Or is there a pre-existing standard JSON schema for the JSON serialized CodeCoverage
"object" that we should look at?
Suppose we're just defining our own schema that is internal for this package, would the desired API look something like:
$json = $codeCoverage->toJson();
$rehydratedCodeCoverage = CodeCoverage::fromJson($json);
such that $codeCoverage
and $rehydratedCodeCoverage
would produce the same exported reports?
Forgive my ignorance if I'm asking questions that don't make sense or have obvious answers.
No worries, your questions make sense.
Is the intention here to have a new JSON format, specific to this package which contains the data needed to instantiate a
CodeCoverage
instance?
Yes, but by now I would prefer to serialize from / unserialize to ProcessedCodeCoverageData
.
$json = $codeCoverage->toJson(); $rehydratedCodeCoverage = CodeCoverage::fromJson($json);
I would prefer to have this in a separate object like so:
final class ProcessedCodeCoverageDataMapper
{
public function toJson(ProcessedCodeCoverageData $data): string
{
}
public function fromJson(string $json): ProcessedCodeCoverageData
{
}
}
such that
$codeCoverage
and$rehydratedCodeCoverage
would produce the same exported reports?
Yes, that would be the idea.
I would prefer to have this in a separate object like so:
Context: I would like to move the merge()
method from the CodeCoverage
object to a separate Merger
object.
The CodeCoverage
object currently has way too many responsibilities and we should at least not add more to it.
Thanks for your clarifications! Yes, that all makes sense and I agree with the suggested approach.
I spent 30 minutes just breaking some ground here trying to find a purchase on it, to drive out the behavior from some tests. I don't expect you to look but I thought I'd share in case you were interested or you think I'm not moving in a good direction.
I've put in a PR as a first step towards resolving this issue here: https://github.com/sebastianbergmann/php-code-coverage/pull/1028
From: https://github.com/sebastianbergmann/php-code-coverage/pull/872#issuecomment-946374189
It should be possible to unserialize a
SebastianBergmann\CodeCoverage\CodeCoverage
object from a different format (like JSON). This could be useful when you want to analyze code coverage aquired by a different way than usingSebastianBergmann\CodeCoverage\CodeCoverage
.At this moment it's only possible create a
SebastianBergmann\CodeCoverage\CodeCoverage
object by using aSebastianBergmann\CodeCoverage\Driver\Driver
or byunserialize()
-ing.I'm not sure if it's possible to change the signature of the
SebastianBergmann\CodeCoverage\CodeCoverage
constructor (by removing or making the driver argument optional). It seems that it's part of the API contract.Another solutions would be to create a Driverless
SebastianBergmann\CodeCoverage\CodeCoverage
class which is used by CodeCoverage (by using inheritance or composition).It might also be possible to inject data in the serialized string and
unserialize()
aSebastianBergmann\CodeCoverage\CodeCoverage
object from that (ieck!).