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

PDF compare TestCase helper #40

Closed
wants to merge 1 commit into from
Closed

Conversation

erikn69
Copy link
Contributor

@erikn69 erikn69 commented Nov 8, 2022

It could be usefull for using this package with PhpUnit test for check / compare two pdf contents

Ideas?

@dealfonso
Copy link
Owner

Hi,
I don't think that document comparison should be incorporated into SAPP as a "trait", or as a "class". Instead, I think it would be enough to incorporate it as a "use case" or an "example" (e.g., enhancing the pdfcompare.php file).

@dealfonso dealfonso self-assigned this Nov 10, 2022
@parallels999
Copy link
Contributor

parallels999 commented Nov 10, 2022

Creo que no afectaría en nada al funcionamiento de SAPP, le agregaría una funcionalidad que no ofrece PHPUNIT y no he visto en otros paquetes
y la verdad no se me había ocurrido hasta que lo vi en Fawno/PDF

@dealfonso
Copy link
Owner

hi,

I have been thinking about this, and I think the method for comparing the documents is not reliable enough. Some minimal changes can cause two equivalent documents not to appear the same, even though they were generated using the same original document and the same sequence of operations. For example, when including the same image in a document, if the SAPP generates those IDs, the image ID may be different.

So I think it is not advisable to include this method as part of SAPP because it can lead to wrong conclusions. It may be sufficient in the context of Fawno/FPDF, but not as a general case.

In any case, thanks for your contribution.

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

Successfully merging this pull request may close these issues.

None yet

3 participants