Skip to content

Implicitly convertion minder to mock.#1

Open
dcorbin wants to merge 1 commit intogreenbar:masterfrom
dcorbin:master
Open

Implicitly convertion minder to mock.#1
dcorbin wants to merge 1 commit intogreenbar:masterfrom
dcorbin:master

Conversation

@dcorbin
Copy link
Collaborator

@dcorbin dcorbin commented Jan 17, 2013

Take a look. The conversion works fine in the real world cases. There was one not-real-world case in the sample where a test was calling the mock directly. You can still do that the way you had it (queue.mock.nextRequest), but I prefer the explicit declaration of the mock.

@kielhodges
Copy link
Collaborator

You confused me when you wrote "I prefer the explicit declaration of the mock." You aren't saying that you don't like the implicit, are you?

But it looks pretty good to me. I think we need to do a little more renaming to go along with it. In the mock definitions, minder is no longer a good alias. Not sure offhand if the generic "self" is appropriate or if there would be a better one. The mock attribute inside of Mock is confusing now. Maybe that should be named subject or something.

On Jan 17, 2013, at 11:49 AM, David Corbin notifications@github.com wrote:

Take a look. The conversion works fine in the real world cases. There was one not-real-world case in the sample where a test was calling the mock directly. You can still do that the way you had it (queue.mock.nextRequest), but I prefer the explicit declaration of the mock.

You can merge this Pull Request by running

git pull https://github.com/dcorbin/replicant master
Or view, comment on, or merge it at:

#1

Commit Summary

Added Implicit conversion of "minder' to mock.
File Changes

M scala/project/plugins.sbt (3)
M scala/project_sbt0.7/build.properties (2)
R scala/src/main/scala/replicant/Mock.scala (9)
R scala/src/test/scala/replicant/MockTest.scala (43)
M scala/src/test/scala/sample/MockGenericRepository.scala (2)
M scala/src/test/scala/sample/MockRequestQueue.scala (2)
M scala/src/test/scala/sample/MockWidgetPainter.scala (2)
M scala/src/test/scala/sample/PaintingTaskTest.scala (13)
Patch Links:

https://github.com/greenbar/replicant/pull/1.patch
https://github.com/greenbar/replicant/pull/1.diff

@dcorbin
Copy link
Collaborator Author

dcorbin commented Jan 18, 2013

No. I meant I prefer to explicitly declare val that is the mock, and assign the minder invoking the implicit conversion. Not do foo.mock.

On Jan 18, 2013, at 8:36 AM, Kiel Hodges notifications@github.com wrote:

You confused me when you wrote "I prefer the explicit declaration of the mock." You aren't saying that you don't like the implicit, are you?

But it looks pretty good to me. I think we need to do a little more renaming to go along with it. In the mock definitions, minder is no longer a good alias. Not sure offhand if the generic "self" is appropriate or if there would be a better one. The mock attribute inside of Mock is confusing now. Maybe that should be named subject or something.

On Jan 17, 2013, at 11:49 AM, David Corbin notifications@github.com wrote:

Take a look. The conversion works fine in the real world cases. There was one not-real-world case in the sample where a test was calling the mock directly. You can still do that the way you had it (queue.mock.nextRequest), but I prefer the explicit declaration of the mock.

You can merge this Pull Request by running

git pull https://github.com/dcorbin/replicant master
Or view, comment on, or merge it at:

#1

Commit Summary

Added Implicit conversion of "minder' to mock.
File Changes

M scala/project/plugins.sbt (3)
M scala/project_sbt0.7/build.properties (2)
R scala/src/main/scala/replicant/Mock.scala (9)
R scala/src/test/scala/replicant/MockTest.scala (43)
M scala/src/test/scala/sample/MockGenericRepository.scala (2)
M scala/src/test/scala/sample/MockRequestQueue.scala (2)
M scala/src/test/scala/sample/MockWidgetPainter.scala (2)
M scala/src/test/scala/sample/PaintingTaskTest.scala (13)
Patch Links:

https://github.com/greenbar/replicant/pull/1.patch
https://github.com/greenbar/replicant/pull/1.diff

Reply to this email directly or view it on GitHub.

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