Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unserialize a SebastianBergmann\CodeCoverage\CodeCoverage object from a different format #874

Open
meeuw opened this issue Oct 22, 2021 · 6 comments

Comments

@meeuw
Copy link

meeuw commented Oct 22, 2021

From: #872 (comment)

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 using SebastianBergmann\CodeCoverage\CodeCoverage.

At this moment it's only possible create a SebastianBergmann\CodeCoverage\CodeCoverage object by using a SebastianBergmann\CodeCoverage\Driver\Driver or by unserialize()-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() a SebastianBergmann\CodeCoverage\CodeCoverage object from that (ieck!).

@AndrewFeeney
Copy link

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?

@sebastianbergmann
Copy link
Owner

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.

@sebastianbergmann
Copy link
Owner

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.

@AndrewFeeney
Copy link

Thanks for your clarifications! Yes, that all makes sense and I agree with the suggested approach.

@AndrewFeeney
Copy link

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.

AndrewFeeney/php-code-coverage@main...implement_processed_code_coverage_data_mapper

@AndrewFeeney
Copy link

I've put in a PR as a first step towards resolving this issue here: #1028

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants