Skip to content

Conversation

Qard
Copy link
Contributor

@Qard Qard commented Jun 26, 2025

I've expanded the SapiBuilder to cover most of the API surface area. This also includes tests to verify the struct fields are set properly.

Do we want some functionality tests too? That'll be a bit more work as it's a bit complicated to set up a fully-functional environment properly.

I also adjusted some more tests that needed the Embed::run(...) wrapper to be ZTS-safe. I can pull those out to a separate PR though, if you prefer.

@Qard Qard force-pushed the expand-sapi-builder branch from af18bff to a17731e Compare June 26, 2025 13:50
@coveralls
Copy link

coveralls commented Jun 26, 2025

Pull Request Test Coverage Report for Build 16006197694

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 66 of 69 (95.65%) changed or added relevant lines in 1 file are covered.
  • 748 unchanged lines in 21 files lost coverage.
  • Overall coverage decreased (-0.3%) to 21.654%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/builders/sapi.rs 66 69 95.65%
Files with Coverage Reduction New Missed Lines %
crates/macros/src/parsing.rs 1 90.0%
src/builders/class.rs 1 57.14%
src/exception.rs 1 90.63%
src/zend/_type.rs 1 69.23%
src/embed/mod.rs 2 66.67%
src/types/iterator.rs 2 55.81%
src/closure.rs 4 9.38%
src/alloc.rs 5 66.67%
src/zend/try_catch.rs 5 80.0%
src/args.rs 7 81.4%
Totals Coverage Status
Change from base Build 15830961013: -0.3%
Covered Lines: 830
Relevant Lines: 3833

💛 - Coveralls

@Qard Qard force-pushed the expand-sapi-builder branch 4 times, most recently from ad760b9 to 8644d1b Compare June 26, 2025 14:48
@Qard Qard force-pushed the expand-sapi-builder branch from 8644d1b to 2533285 Compare June 26, 2025 14:58
@Qard
Copy link
Contributor Author

Qard commented Jun 30, 2025

@Xenira This is the main SAPI-related change I need to be able to drop my fork. How does this look? Anything more you'd want from it?

@Xenira
Copy link
Collaborator

Xenira commented Jul 1, 2025

@Qard Thank you. Was on vacation the past few days and need to catch up as a lot happened while I was away. Will have a look as soon as possible.

@Qard
Copy link
Contributor Author

Qard commented Jul 1, 2025

No worries. I hope it was a nice vacation! 🙂

@Xenira Xenira changed the title feat: expand SapiBuilder feat(sapi): expand SapiBuilder Jul 1, 2025
Copy link
Collaborator

@Xenira Xenira left a comment

Choose a reason for hiding this comment

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

This looks really good. Thank you! Only have the nitpickiest of nitpicky comments.

@Xenira Xenira merged commit bed5521 into davidcole1340:master Jul 1, 2025
55 checks passed
@davidcole1340 davidcole1340 mentioned this pull request Jul 1, 2025
@Qard Qard deleted the expand-sapi-builder branch July 3, 2025 11:45
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