Selectmenu: unit tests #536

Merged
merged 10 commits into from Jan 11, 2012

Conversation

Projects
None yet
3 participants
Contributor

danwellman commented Nov 25, 2011

Added unit tests for options and events

Member

fnagel commented Nov 28, 2011

I think the event tests are pretty nice so far, but we should extend the change event test to check if there was really a new option selected (change vs. select callback). Currently both checks do the same.

It seems to me that your options tests just checks if the default values are correct. This is done by selectmenu_defaults.js afaik. I assume that for example appendTo option test should really check if the menu is injected to the defined element (like its done in autocomplete options unit test).

As I already told you Im new to jQuery unit tests, so Im not sure if my thoughts (especially regarding your options tests) are correct. Perhaps @jzaefferer could take a look and give us some advises?

Contributor

danwellman commented Nov 28, 2011

@fnagel Agreed, will beef up the events tests, I thought of a couple more to add over the weekend too, will include these in the next commit

I will update the option tests by removing the default value tests, and adding more tests after changing options. E.g. the 'appendTo another element' test checks that a selector is accepted as the value, but I will add another test that checks the menu has actually been appended to another element...

Contributor

danwellman commented Dec 1, 2011

Hi Felix, I removed the duplicate tests in options and added the ones you suggested. I also updated the events tests; since Joern's update/fix I have been able to remove the conditional checking in most of the events tests :D

Contributor

danwellman commented Dec 1, 2011

Hi Felix, I updated the tests, the new versions are showing in the pull request :)
Let me know if anything further is needed for them.
Dan

Date: Sun, 27 Nov 2011 16:36:32 -0800
From: reply@reply.github.com
To: danwellman@hotmail.com
Subject: Re: [jquery-ui] Selectmenu (#536)

I think the event tests are pretty nice so far, but we should extend the change event test to check if there was really a new option selected (change vs. select callback). Currently both checks do the same.

It seems to me that your options tests just checks if the default values are correct. This is done by selectmenu_defaults.js afaik. I assume that for example appendTo option test should really check if the menu is injected to the defined element (like its done in autocomplete options unit test).

As I already told you Im new to jQuery unit tests, so Im not sure if my thoughts (especially regarding your options tests) are correct. Perhaps @jzaefferer could take a look and give us some advises?


Reply to this email directly or view it on GitHub:
#536 (comment)

Member

fnagel commented Dec 1, 2011

I talked to Jörn about our next steps:

  • complete unit tests as good as we can
  • pull your changes
  • wait till Selectmenu is again fully compatible with Menu
  • Jörn takes a look
Contributor

danwellman commented Dec 1, 2011

Ok sounds cool, I will keep updating the tests when I get time to add your suggestions and will add any new tests I think beneficial :)
Dan

Date: Thu, 1 Dec 2011 10:51:12 -0800
From: reply@reply.github.com
To: danwellman@hotmail.com
Subject: Re: [jquery-ui] Selectmenu (#536)

I talked to Jrn about our next steps:

  • wait till Selectmenu is again fully compatible with Menu
  • complete unit tests as good as we can
  • pull your changes
  • Jrn takes a look

Reply to this email directly or view it on GitHub:
#536 (comment)

Contributor

danwellman commented Dec 13, 2011

Hey Felix,

I just pushed some updates, it'll show up in the pull request automatically?

I had a couple of issues testing the CSS as it was behaving completely as expected (i.e. as it does on the selectmenu wiki page demos), but I added a couple of tests and tidied a couple of things up...

Let me know what you think :)

Member

fnagel commented Dec 21, 2011

It will show up -- just above your last comment :-)

Thanks, looks great so far!

fnagel added a commit that referenced this pull request Jan 11, 2012

Merge pull request #536 from danwellman/selectmenu
Selectmenu: additional unit tests

@fnagel fnagel merged commit db4acf6 into jquery:selectmenu Jan 11, 2012

Owner

scottgonzalez commented on 63d77ae Dec 11, 2013

@danwellman Can you please sign our CLA?

Contributor

danwellman replied Dec 11, 2013

Done :)

Owner

scottgonzalez replied Dec 11, 2013

Thanks.

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