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

test the reader with the writer #2745

Merged
merged 1 commit into from
Feb 15, 2025

Conversation

MichaelPFrey
Copy link
Contributor

@MichaelPFrey MichaelPFrey commented Feb 9, 2025

Description

I think that currently, there are only few good tests for the reader. (see for e.g. #2700 where the reader and writer where modified, but only the writer is actually covered by a test)

Creating and adding artifacts (docx and odt) is not ideal, as they are hard to verify and will contain random other artifacts.

By dynamically creating the test-file using the writer and reading it back in with the reader, we can simply test the reader against the writer.

In general, a WYSWYG word processor is assumed to read and save files without loosing anything. For a project like PHPOffice, the focus is stronger on the writer then the reader. With this setup, it is a lot easier to work on the reader, as a test driven development purely in PHP gets enabled - the need to use a word processor for verification gets significantly reduced.

There is also simply charm in self testing, as a simple to read test makes a literal "end to end" test, covering a lot of complex code with a few lines of simple code.

Fixes # (issue)

Checklist:

  • My CI is 🟢
  • I have covered by unit tests my new code (check build/coverage for coverage report)
  • I have updated the documentation to describe the changes
  • I have updated the changelog

This are literal test. They do not effect the production code. So the checklist does not directly apply.

@coveralls
Copy link

coveralls commented Feb 9, 2025

Coverage Status

coverage: 96.765% (+0.008%) from 96.757%
when pulling a902537 on MichaelPFrey:mfr-2025-02-09
into 5c84adf on PHPOffice:master.

@Progi1984
Copy link
Member

@MichaelPFrey Could you update the changelog, please ?

@Progi1984 Progi1984 added the Status: Waiting for feedback Question has been asked, waiting for response from PR author label Feb 12, 2025
@Progi1984 Progi1984 removed the Status: Waiting for feedback Question has been asked, waiting for response from PR author label Feb 15, 2025
@Progi1984 Progi1984 added this to the 1.4.0 milestone Feb 15, 2025
@Progi1984 Progi1984 merged commit cdf3d31 into PHPOffice:master Feb 15, 2025
30 checks passed
@Progi1984
Copy link
Member

@MichaelPFrey Thank you for your contribution 🥳

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

Successfully merging this pull request may close these issues.

3 participants