Skip to content

Add tests and GitHub actions #6

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

Merged
merged 6 commits into from
Nov 24, 2024
Merged

Conversation

duboism
Copy link
Contributor

@duboism duboism commented Oct 5, 2024

This PR modifies the test scripts so they can be run on modern installation. Also it set up github actions.

Technical details

  • We only test PHP 5.6 to 7.4 because the code is not compatible with PHP 8.
  • We need to install MicroSoft's Verdana font (for test using text). This is legal (see https://corefonts.sourceforge.net/).
  • Postscript output is not tested because I have issues with the Helvetica font. I think it's doable.
  • PDF output is not tested because it requires the old PDFlib extension so there is 0 chances this works (it should be redone in another PR).

@duboism
Copy link
Contributor Author

duboism commented Oct 24, 2024

Any news ?

@ashnazg ashnazg self-assigned this Nov 24, 2024
@@ -27,7 +27,7 @@
* @link http://pear.php.net/package/Image_Canvas
*/

require_once 'Image/Canvas.php';
require_once 'vendor/autoload.php';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't let this stop a merge now, but I'd prefer a more flexible approach here that doesn't force a composer installation before someone could run the testsuite. Just a simple existence check on the vendor autoload in order to require it, and fall back to the old PEAR-style require otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good remark but I'm not sure I understand all the details of it. Just testing the existence of vendor/autoload.php would be enough ? Do you know a package that does that (so I can take inspiration from it) ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (file_exists('vendor/autoload.php')) {
    // use composer if available
    require_once 'vendor/autoload.php';
} else {
    // otherwise rely on classic PEAR include_path
    require_once 'Image/Canvas.php';
}

@ashnazg ashnazg merged commit 06cbacd into pear:master Nov 24, 2024
@duboism
Copy link
Contributor Author

duboism commented Nov 24, 2024

Thanks @ashnazg for merging this.

@duboism duboism deleted the test_and_github_actions branch November 24, 2024 21:04
@Neustradamus
Copy link

Thanks to @duboism for this PR and @ashnazg for merging!
Good job!

Can you add "Issues" section?

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.

3 participants