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

Use AspectMock in tests #419

Closed
wants to merge 1 commit into from
Closed

Conversation

ob-stripe
Copy link
Contributor

r? @brandur-stripe
cc @stripe/api-libraries

Adds support for the AspectMock library in tests.

AspectMock allows for some things that PHPUnit itself cannot handle, most notable mocking/stubbing static methods. Since we use a lot of static methods throughout the library, we will be able to use AspectMock to write better tests.

@ob-stripe
Copy link
Contributor Author

Argh. Looks like AspectMock (or rather, the Go! AOP framework dependency) does not support HHVM: goaop/framework#143 (comment).

@brandur-stripe
Copy link
Contributor

brandur-stripe commented Jan 16, 2018

Doh!

@ob-stripe I left some messages on Slack, but it does seem like unfortunately cross compatibility between PHP and HHVM is not long for this world, even if it's the libraries that are more intertwined with internals (i.e., like a mocking library) that need to drop compatibility first.

Let me know what you think, but I have a slight preference for keeping HHVM support around until it starts to cause more acute pain (and hopefully until after most other projects have dropped it). This would unfortunately mean foregoing more advanced mocking libraries for the time being.

The other alternative is that instead of trying to improve our testing, we could start refactoring in the direction of non-static methods. This would be a little too much API churn probably, but if we were starting from zero, it'd probably be the superior design.

@ob-stripe
Copy link
Contributor Author

The other alternative is that instead of trying to improve our testing, we could start refactoring in the direction of non-static methods. This would be a little too much API churn probably, but if we were starting from zero, it'd probably be the superior design.

Definitely agreed, though it will very likely require some breaking changes. (Though if we ever are to tackle #124, there's probably no way around it anyway.)

Anyway, closing for now. As much as I'd have liked to use AspectMock for testing some new features, I'll manage without it.

@ob-stripe ob-stripe closed this Jan 17, 2018
@ob-stripe ob-stripe deleted the ob-use-aspectmock branch January 17, 2018 09:29
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.

2 participants