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

Add tests for the sortUsers method #197

Merged
merged 1 commit into from
Mar 16, 2016
Merged

Add tests for the sortUsers method #197

merged 1 commit into from
Mar 16, 2016

Conversation

astorije
Copy link
Member

Behold... tests!

I'm usually not too fond of having placeholder code until something happens, but considering @xPaw is making good progress by himself on #167, I thought I would give a minor hand by providing the lines to change to have these tests running in his PR.
And as a matter of fact... these tests are currently failing on #167 :D

Nevermind, removed the placeholders, will re-add when time comes.

I hope you like 'em, let me tell you this is the beginning of a whole lot of PRs about testing I'm planning to submit :-)

(Yes, I'm adding Chai in the devDependencies, but it's really worth it!)


describe("Chan", function() {
describe("#sortUsers()", function() {
// TODO Use this version instead when switching to irc-framework
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove these TODOs, it's unnecessary. After this PR is merged, just update the tests in ir-framework branch.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed in 1715059.

@xPaw
Copy link
Member

xPaw commented Mar 15, 2016

Add a couple of users with different cases in the name, and a couple of special symbols to test case insensitive sorting too.

@astorije astorije force-pushed the astorije/test-sort-users branch 2 times, most recently from 1715059 to a6b452d Compare March 16, 2016 05:00
@astorije
Copy link
Member Author

@xPaw, good suggestions! Added in a6b452d. I hope it can go now :-)

@astorije astorije added the Type: Feature Tickets that describe a desired feature or PRs that add them to the project. label Mar 16, 2016
@astorije
Copy link
Member Author

Also, should test file name match tested file name (like currently)? Or something.js and somethingSuite.js? Or... ?
I don't have a strong opinion on this, as long as it ships :-)

xPaw added a commit that referenced this pull request Mar 16, 2016
@xPaw xPaw merged commit 9ef348e into master Mar 16, 2016
@xPaw xPaw deleted the astorije/test-sort-users branch March 16, 2016 08:58
@astorije astorije added this to the 1.4.0 milestone Oct 8, 2016
@xPaw xPaw removed their assignment Mar 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature Tickets that describe a desired feature or PRs that add them to the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants