Skip to content

Port JSSpec and qunit tests over to Jasmine#718

Merged
SteveSanderson merged 10 commits into
knockout:masterfrom
chrisprice:jasmine
Nov 20, 2012
Merged

Port JSSpec and qunit tests over to Jasmine#718
SteveSanderson merged 10 commits into
knockout:masterfrom
chrisprice:jasmine

Conversation

@chrisprice

Copy link
Copy Markdown
Contributor

As a pre-cursor to running some tests out of the browser for #712, something unsupported by JSSpec, I've had a go at porting all of the test specs over to use Jasmine. This has a number of benefits -

  • Speed - the tests now execute in about 1/10th of the time (using phantom on my machine)
  • Async - as jasmine supports async specs, qunit can be removed and the async tests can be run as part of CI
  • Browserless - jasmine will happily run in a browserless environment
  • Maintenance - jasmine has been steadily maintained for the last 4 years

The jasmine API is very similar to the JSSpec one so most of the porting isn't very controversial. However, there are three commits worth drawing attention to -

I've run the tests successfully on all the browsers I have access to.

I hope this change isn't too controversial, I noticed on the mailing list that lots of people were using jasmine for their own tests.

Chris

@SteveSanderson

Copy link
Copy Markdown
Contributor

Wow, thanks so much for doing this!

It might take a few days for us to go through this and be sure we're ready to merge it in, but I'll be so pleased to switch over to Jasmine (and not have the weird JsSpec/QUnit combination).

@rniemeyer

Copy link
Copy Markdown
Member

This is great! I use Jasmine for all of my other repositories and am very happy with it. I think that it will be good for us to use it. I think that we could get good use out of Jasmine's spies as well to simplify some cases, but a 1:1 port of the old tests seems like the best place to start.

@mbest

mbest commented Nov 17, 2012

Copy link
Copy Markdown
Member

This is a great change! I've created a local branch for this and made some changes: master...718-Jasmine-tests

@SteveSanderson

Copy link
Copy Markdown
Contributor

Fantastic. I've been through this in some detail and still think it looks great :) I added a couple more tweaks to the local branch that Michael created, but nothing significant.

Michael, could you clarify the point about jasmine.extensions.css (runner.html line 17). Once that is resolved, as far as I am concerned this is all ready to merge into master.

@mbest

mbest commented Nov 20, 2012

Copy link
Copy Markdown
Member

Sorry I forgot to include it. It's just a simple tweak to hide the html used by the tests.

@SteveSanderson SteveSanderson merged commit c236572 into knockout:master Nov 20, 2012
@SteveSanderson

Copy link
Copy Markdown
Contributor

OK great, merged in.

Thanks again Chris, this is really valuable!

@rniemeyer

Copy link
Copy Markdown
Member

yay! Nice work @chrisprice :)

@chrisprice

Copy link
Copy Markdown
Contributor Author

Awesome, thanks gents! And now it's in, no one will ever know how much horrendous regex went into this change...opps... :)

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.

4 participants