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

Experiment with ShoesSpec #115

Merged
merged 32 commits into from Sep 22, 2012
Merged

Experiment with ShoesSpec #115

merged 32 commits into from Sep 22, 2012

Conversation

wasnotrice
Copy link
Member

We have discussed using the specs in spec/shoes as a sort of ShoesSpec, like RubySpec for Shoes.

This commit modifies the spec:swt Rake task so that, in addition to the running the swt-specific tasks it also runs the Shoes DSL specs with the Swt backend.

Currently almost all of the Shoes DSL specs fail with the Swt backend. Mostly (at this point anyway) this is because of mock objects receiving unexpected messages.

Let's have a conversation about how to make these specs work well. @alindeman, I think you had some ideas about this at one point?

@travisbot
Copy link

This pull request fails (merged 696cf68 into 70ffb6a).

@davorb
Copy link
Member

davorb commented Sep 7, 2012

I think we're going to have to start moving a lot of the mocking stuff from the spec files (where you normally have them with RSpec) to the actual mock files, for this to work. Also, I suspect that we're going to need a lot of def method_missing()s to pull this off.

@wasnotrice
Copy link
Member Author

I played with it a little today and I think it'll be fine. Making the gui mock into a null object made almost all of the Line specs pass with the Swt backend. I'll push that code soon.

Fundamentally, we just have to make sure that the backend classes share the same interface, and then only use that interface in the specs. Then the objects should duck type and make us happy. We just don't have a compiler to help us ;)

@pjfitzgibbons
Copy link
Member

Are you sure w/ the "null object" that you have no false-positives in the
testing?

Just being wary of the fabled "null object" in general. Could be ok here.
You tell me?

Kindest Regards,
Peter Fitzgibbons
(847) 859-9550
Email: peter.fitzgibbons@gmail.com
IM GTalk: peter.fitzgibbons
IM AOL: peter.fitzgibbons@gmail.com

On Fri, Sep 7, 2012 at 4:25 PM, Eric Watson notifications@github.comwrote:

I played with it a little today and I think it'll be fine. Making the gui
mock into a null object made almost all of the Line specs pass with the Swt
backend. I'll push that code soon.

Fundamentally, we just have to make sure that the backend classes share
the same interface, and then only use that interface in the specs. Then the
objects should duck type and make us happy. We just don't have a compiler
to help us ;)


Reply to this email directly or view it on GitHubhttps://github.com//pull/115#issuecomment-8379465.

@wasnotrice
Copy link
Member Author

Are you sure w/ the "null object" that you have no false-positives in the testing?

Well, in this particular case, I'm pretty sure, since I have been testing it manually in apps. However, in general, I am skeptical of the null object. This was just an experiment to see how little code I would have to change to get specs running (answer: very little). I'll try substituting "real" objects for the test doubles next.

Thanks for keeping us honest :)

@PragTob
Copy link
Member

PragTob commented Sep 15, 2012

I'm also skeptical of the null object, although it is highly dependent on the context... Using straight up real objects could be cumbersome as well since probably that would really create gui elements while running the test suite and make it noticeably slower... So I don't know :-/

@wasnotrice
Copy link
Member Author

Ok. The point of a ShoesSpec is to help us be confident that all of the backends behave "the same". So it makes sense that we should use as much of the real stack as possible. I have done only a brief experiment with using real objects in place of mocks (usually its the App objects), but it will work fine. Not _too_ much more work than the null object route. It doesn't seem too slow, either ;)

A great side effect of this exercise is that it exposes a bunch of places where our code is too tightly coupled. Listen to the tests!

@PragTob
Copy link
Member

PragTob commented Sep 15, 2012

Sounds good :-)

Would have to try it out myself though. I'm not too much concerned about speed though... we can worry about that when it becomes a problem. Because generally, I'm more happy with tests that really test something than fast tests.

Point taken, thanks Eric!

@wasnotrice
Copy link
Member Author

If you want to check out the specs, I've pushed up to my shoesspec branch the two strategies.

  1. Specs for Line are implemented using the null object strategy
  2. Specs for Oval are implemented using real Shoes objects. This means that when you run rake spec:shoes, you get Mock backend classes, and when you run rake spec:swt, you get Swt backend classes.

I think strategy 2 (real Shoes objects) is the way to go (not much harder), but check it out yourself and see what you think. Remember that,

  • on this branch, rake spec:shoes acts as usual, but rake spec:swt runs specs in spec/shoes _and_ spec/swt_shoes.
  • thanks to @pjfitzgibbons, you can run rake spec:swt[Oval] if you just want to see the specs that exercise Oval (you might want to do this, because there are currently 269 failures when you run all of the specs).

@wasnotrice
Copy link
Member Author

Just an update: I'm now down to only 59 failures out of 561 specs when I run the whole suite with the Swt backend. The specs do take about 10 times as long to run as when they're run with the mock backend :(

Still, in my limited number of runs, that run time is only 10 to 23 seconds, vs 1 to 2 seconds for the mock suite.

@pjfitzgibbons
Copy link
Member

HI Eric. Where is your code again? I want to see and understand.

Peter Fitzgibbons
(847) 859-9550
Email: peter.fitzgibbons@gmail.com
IM GTalk: peter.fitzgibbons
IM AOL: peter.fitzgibbons@gmail.com

On Sat, Sep 15, 2012 at 11:54 PM, Eric Watson notifications@github.comwrote:

Just an update: I'm now down to only 59 failures out of 561 specs when I
run the whole suite with the Swt backend. The specs do take about 10 times
as long to run as when they're run with the mock backend :(

Still, in my limited number of runs, that run time is only 10 to 23
seconds, vs 1 to 2 seconds for the mock suite.


Reply to this email directly or view it on GitHubhttps://github.com//pull/115#issuecomment-8592268.

@steveklabnik
Copy link
Member

The specs do take about 10 times as long to run as when they're run with the mock backend :(

Yep, that's the classic problem with integration tests. Don't mean it's not worth it, but thank $DIETY for Travis!

@wasnotrice
Copy link
Member Author

@pjfitzgibbons it's on the shoesspec branch of wasnotrice/shoes4. This is the latest commit.

@wasnotrice
Copy link
Member Author

@pjfitzgibbons Actually, just view this pull request on github and you can see all of the relevant commits ;)

@wasnotrice
Copy link
Member Author

@davorb I'm updating TextBlock specs to run using Swt backend. It seems to me that the "font styles" specs are not actually implemented in Swt, so I should mark them as not implemented in Swt. Does that sound right?

@wasnotrice
Copy link
Member Author

Current status: getting closer!

$ rake spec:swt

[snip]

Finished in 9.64 seconds
564 examples, 26 failures

@steveklabnik
Copy link
Member

:D

@davorb
Copy link
Member

davorb commented Sep 17, 2012

@wasnotrice

Sorry for the late response! Been really busy...

Yeah, that's fine. In fact, I did the same thing when I converted
TextBlock to the "new" (gc.drawFont) format.

I'll see if I can push that code to the repo tonight...

Davor Babic
davor@davor.se

Eric Watson wrote:

@davorb https://github.com/davorb I'm updating TextBlock specs to run
using Swt backend. It seems to me that the "font styles" specs are not
actually implemented in Swt, so I should mark them as not implemented in
Swt. Does that sound right?


Reply to this email directly or view it on GitHub
#115 (comment).

Also  add MSpec-style guard to only run a group of specs when using
a given backend
rake spec:swt              Run all Swt backend isolation specs and all
                           integration specs using the Swt backend
rake spec:swt:integration  Run all integration specs using the Swt
                           backend
rake spec:swt:isolation    Run all Swt backend isolation specs

When passed a class/module name as an argument, each of these commands
(like the other spec commands) will run only those specs that mention
the argument in the description, e.g. rake spec:swt[Arc] will run both
isolation and integration specs for Arc.
@wasnotrice
Copy link
Member Author

As of 634a1cd, all Shoes specs pass using the Swt backend as well as the mock backend. I think we have a ShoesSpec!

I have updated the rake commands so you can choose which specs to run. Now we have:

rake spec:swt              Run all Swt backend isolation specs and all
                           integration specs using the Swt backend
rake spec:swt:integration  Run all integration specs using the Swt
                           backend
rake spec:swt:isolation    Run all Swt backend isolation specs

When passed a class/module name as an argument, each of these commands
(like the other spec commands) will run only those specs that mention
the argument in the description, e.g. rake spec:swt[Arc] will run both
isolation and integration specs for Arc.

Give it a whirl. What do you think?

@PragTob
Copy link
Member

PragTob commented Sep 21, 2012

Wow awesome! =)

I'll give it a whirl either today or tomorrow! Thank you @wasnotrice !

@pjfitzgibbons
Copy link
Member

yep, I got green!
Great work Eric

Peter Fitzgibbons
(847) 859-9550
Email: peter.fitzgibbons@gmail.com
IM GTalk: peter.fitzgibbons
IM AOL: peter.fitzgibbons@gmail.com

On Fri, Sep 21, 2012 at 5:25 AM, Tobias Pfeiffer
notifications@github.comwrote:

Wow awesome! =)

I'll give it a whirl either today or tomorrow! Thank you @wasnotricehttps://github.com/wasnotrice!


Reply to this email directly or view it on GitHubhttps://github.com//pull/115#issuecomment-8760642.

ashbb added a commit that referenced this pull request Sep 22, 2012
@ashbb ashbb merged commit ed027a9 into shoes:master Sep 22, 2012
@ashbb
Copy link
Member

ashbb commented Sep 22, 2012

@wasnotrice Great work! Thanks!

@PragTob
Copy link
Member

PragTob commented Sep 23, 2012

Later then promised, but works beautifully and is still fast. Thank you so much!

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.

None yet

7 participants