Implement spies for get/set functions on accessor properties

[smacker]

I tried to implement #943

Added spyOnProperty method as @slackersoft recommended.

[brunoskonrad]

❤️

[smacker]

Please, take a look.
I updated code to fix merge conflict with #1036
And added same test for spyOnProperty.

[henrahmagix]

@smacker I have fixed your PR so spyOnProperty is available in specs: see smacker#1 Merging that should update this PR so hopefully we can get this merged, I really want to use this feature =)

[smacker]

Thanks a lot! Merged!

[celluj34]

It looks like this has been merged, but the PR is still open.

Also, I could really use this functionality. Any word on when it would be available in nuget/npm?

[henrahmagix]

@celluj34 My pull-request into this was merged.

I agree, hearing a release timeline would be really nice!

[gantaa]

Release this feature please. Thanks for the hard work

[erwinverdonk]

I am just passing by this issue and notice that you are already waiting months for this to be released. However as you can see at the bottom of this page (in desktop view) the PR branch has conflicts that needs resolving. I take that this is the reason why it is not merged yet, although not well communicated. So maybe it is an idea that either the PR author or someone else creates a new branch from master, applies changes again, resolve any conflicts, and do another PR for this new branch? Maybe then it gives the maintainers more motivation to actually merge it, as it should then be conflict free :).

[henrahmagix]

@erwinverdonk When smacker#2 is merged the conflicts here should be fixed =)

[smacker]

I merged it, but looks like jasmine team isn't interested in this feature. :(

[erwinverdonk]

Excellent work guys! Lets at least try to get a response from the Jasmin team. I will try to reach out to the maintainers for this.

[erwinverdonk]

Ok, I just sent this email to the maintainers (amavisca, jboyens and vinsonchuong):

Dear maintainers of Jasmine,

To be short about this; this Pull Request was very old and had not received any feedback. Because of the time that had passed there were conflicts introduced that needed fixing before it could be pulled in. The community made the effort to branch off the latest code base, apply the changes for the PR again and add it as a new PR.

It can be found here: #1203 (comment)
This new PR is also merged in the old PR here: #1008 (comment)

The only question now is what is your feedback for this Pull Request? Maybe we need to change something or you need to discuss this internally. Would you please have a look? Please let us know :).

Kind Regards,

Erwin Verdonk

[maxlk]

At first, I thought that it's a great feature, but now I think that it's not usable.
The first reason, it's too verbose:

expect(Object.getOwnPropertyDescriptor(subject, 'spiedProperty').set).toHaveBeenCalled()

The second and the main reason, it's definitely a bad practice to mark property configurable just for testing purpose. If design doesn't require the property to be configurable, it should not be. And it's very serious obstacle, that makes this feature almost useless.

[rbnzdave]

For a start, where does verbose come into it, they were asking for a spyOnProperty([my object path],[the property]) so would have used a variable name, and then there is the fact that you wouldnt add this to your existing codebase just for testing purposes, you would use it to check if fields in your mock services/objects were accessed by the object you are testing, which is the exact reason I'm trying to use it.

[henrahmagix]

Thanks @slackersoft!

[booleanbetrayal]

I just used this today and it was glorious. =]