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

1.0 Discussion #288

Closed
davedevelopment opened this Issue Feb 24, 2014 · 22 comments

Comments

Projects
None yet
5 participants
@davedevelopment
Collaborator

davedevelopment commented Feb 24, 2014

Just creating this as a place holder for discussions pertaining to a 1.0, will formalise something eventually.

Things I've been thinking about:

  • Spies
    • Rough implementation in #283
    • API needs discussing and finalising
  • Docs
  • API
    • Whole public API needs examining and finalising
  • Stubs vs Mocks behaviours
    • IMO stubs should always be byDefault, i.e. you can override them
    • I would love to break the API completely, so that shouldReceive defaults to once, rather than zeroOrMoreTimes, along with a stub method, as I think the language is in conflict, but I think that's probably too much of a BC break.
  • Issues
    • We need to fix and/or close issues as necessary
  • User Feedback
    • Need to speak to user base to see what we're missing
  • API Verifying mocks
    • Above and beyond allowMockingNonExistentMethods
    • Checks against method signature (for primitives and arg count etc)
    • Checks against docblock return type
    • Could be (configurably) silent when class/interface doesn't exist
    • Could be (configurably) silent when method doesn't exist

Newly added:

  • PHP version support
    • PHP 5.3.x is EOL
    • See #297
  • Coding Standards
  • Review state of matchers #129
    • I think strictness with regards to objects with($obj) and with(mustBe($obj)) are backwards compared to primitives.
@padraic

This comment has been minimized.

Member

padraic commented Feb 24, 2014

I should be able to be pitch in soon. Finally ;). Running a team for over a year with a minimum of staff does not do wonders for your free time or stress levels. At the moment, I'm on a security crusade so I have a chunk of work to do for Composer and some other bits in private, but I'll start swinging through the open issues as I have time.

I haven't checked, but it's possible we might be able to leverage off the existing Markdown and use something like pandoc (http://johnmacfarlane.net/pandoc/) to convert pages to restructuredText as part of a build process.

@GrahamCampbell

This comment has been minimized.

Contributor

GrahamCampbell commented Feb 25, 2014

Can we follow PSR-1 for method names please. PSR-1 says they should be camel cased. This would mean renaming 31 public methods and 10 private or protected methods. Also, we get rid of the underscore prefixing on private/protected methods/variables?

@davedevelopment

This comment has been minimized.

Collaborator

davedevelopment commented Feb 25, 2014

@GrahamCampbell I don't see why not, might want to hang fire on it until we finalise the API, any changes we make at that stage can make the necessary changes.

@GrahamCampbell

This comment has been minimized.

Contributor

GrahamCampbell commented Feb 25, 2014

Sure. I am can make a pull in a few weeks/months for that transition if you like?

@padraic

This comment has been minimized.

Member

padraic commented Feb 25, 2014

@GrahamCampbell Most of the PSR-1 issues are the prefixed methods for mocks, e.g. mockery_reservedMethodName() - it was probably overkill on my part. Underlined methods are allowable under PSR-1. Yes, I know phpcs gives a warning, but a warning is not an error ;).

@GrahamCampbell

This comment has been minimized.

Contributor

GrahamCampbell commented Feb 25, 2014

Ok. Sure. :)

@padraic padraic added the internals label Feb 27, 2014

@davedevelopment

This comment has been minimized.

Collaborator

davedevelopment commented Mar 12, 2014

Updated with php version support and coding standards.

@davedevelopment

This comment has been minimized.

Collaborator

davedevelopment commented Apr 1, 2014

Added note about stubs vs mocks, specifically with regards to byDefault, see #307 and my comments on http://everzet.com/post/72910908762/conceptual-difference-between-mockery-and-prophecy

@fedecarg

This comment has been minimized.

fedecarg commented Apr 2, 2014

Hi Dave/Paddy, what about stubs checking the return value but not the data type, do you see this as a problem when it comes to state verification in PHP: https://gist.github.com/fedecarg/10223193

@davedevelopment

This comment has been minimized.

Collaborator

davedevelopment commented Apr 2, 2014

@fedecarg I don't know if I see it as a problem per se (others might), but I definitely see it as something that would be nice to have.

We already have the allowMockingNonExistentMethods config setting, but I'd definitely like to have other functionality such as what you have mentioned along side it. More importantly though, I'd like to have the option for it more like a sniffer, so the violations can be just reported. We can then go on to make best practice recommendations as well. But time, effort etc.

@fedecarg

This comment has been minimized.

fedecarg commented Apr 5, 2014

Hi @davedevelopment Yes, my 2 biggest concerns at the moment are:

  1. Behaviour verification: Mockery cannot provide guarantees about what is being verified because it uses normal doubles instead of verifying doubles (see RSpec Mocks 3.0).

  2. State verification: Mockery cannot provide guarantees about what is being returned (in php you can dynamically type the return value within the body of the method).

@davedevelopment

This comment has been minimized.

Collaborator

davedevelopment commented Apr 6, 2014

@fedecarg roger, added a section about "API verifying mocks" for discussion.

@davedevelopment

This comment has been minimized.

Collaborator

davedevelopment commented May 1, 2014

Added note about argument matchers #129

@aik099

This comment has been minimized.

Contributor

aik099 commented Jun 25, 2014

PHP 5.3.x is EOL

What that means? The Mockery 1.0 will require PHP 5.4 to work. If so, then why?

@davedevelopment

This comment has been minimized.

Collaborator

davedevelopment commented Jun 25, 2014

PHP 5.3.x is EOL

What that means? The Mockery 1.0 will require PHP 5.4 to work. If so, then why?

Quite possibly. PHP 5.3.x is older, slower and unsupported by the PHP team. The longer we support it, the more likely we are to accrue technical debt.

@aik099

This comment has been minimized.

Contributor

aik099 commented Jun 25, 2014

But how often do we hit PHP 5.3 problems when using Mockery and are they really a problem for all users for just for some special use cases?

@davedevelopment

This comment has been minimized.

Collaborator

davedevelopment commented Jun 25, 2014

@aik099 maybe, but there are many things to consider. Take the recent messing about with the unserialize trick. That functionality is going to end up being different across three versions of PHP (5.3,5.4,5.6) and it gets difficult, boring and annoying to maintain. People will always be able to continue using older versions of mockery and people could always maintain a 0.9 branch that is php 5.3 compatible.

@aik099

This comment has been minimized.

Contributor

aik099 commented Jun 25, 2014

People will always be able to continue using older versions of mockery and people could always maintain a 0.9 branch that is php 5.3 compatible.

That's good news. What I'm worried a bit is a feature dispersion between Mockery 0.9.x and Mockery 1.0. For example the ->byDefault() call that will happen automatically might be useful in 0.9.x and it's not a large BC break.

@davedevelopment

This comment has been minimized.

Collaborator

davedevelopment commented Jun 25, 2014

That's good news. What I'm worried a bit is a feature dispersion between Mockery 0.9.x and Mockery 1.0. For example the ->byDefault() call that will happen automatically might be useful in 0.9.x and it's not a large BC break.

Yeah.

I'm not saying it will definitely happen though, phpunit nearly did it for 3.8, but reverted the decision in the end, though I'm not sure why.

@padraic

This comment has been minimized.

Member

padraic commented Feb 13, 2015

@davedevelopment Should we consider moving the list to a set of separate issues under a new label?

@davedevelopment

This comment has been minimized.

Collaborator

davedevelopment commented Feb 13, 2015

@padraic I think that would be a good idea, if indeed we agree on a list of things. This really was just me brain dumping things :)

@padraic

This comment has been minimized.

Member

padraic commented Feb 13, 2015

I did a few new ones already, being full of sugar and caffeine ;). I'd suggest creating them as RFC issues with the Towards 1.0.0 label and we'll see how they play out.

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