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 more automated tests #903

Merged
merged 3 commits into from
Jun 5, 2015
Merged

Conversation

michaelansel
Copy link
Collaborator

Another batch of tests covering:

  • Robot: listener registration (complete)
  • Robot: script loading (incomplete, just loadFile)
  • Brain: everything (complete)

user4 = @brain.userForId('FOUR')
expect(@brain.userForId('four')).to.not.equal(user4)

# Cannot understand why this behavior is needed, but adding a test case
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think this can be removed, but I figure it is better to have the tests now and then remove them later.

@michaelansel
Copy link
Collaborator Author

@technicalpickles any objection to merging these tests?

@technicalpickles
Copy link
Member

@michaelansel no objections, but I would like to do a pass through to make sure I can understand what's going on 😁

@michaelansel
Copy link
Collaborator Author

Been a couple months. Poking this again...

@bkeepers
Copy link
Contributor

bkeepers commented Jun 4, 2015

I just merged master locally and they all still pass. Since they are tests and won't affect any APIs, they can easily be change later. I vote 👍.

@sdimkov
Copy link
Contributor

sdimkov commented Jun 4, 2015

👍

michaelansel added a commit that referenced this pull request Jun 5, 2015
@michaelansel michaelansel merged commit 2321c24 into hubotio:master Jun 5, 2015
@michaelansel michaelansel deleted the automated-tests branch June 5, 2015 19:08
@technicalpickles technicalpickles mentioned this pull request Jun 5, 2015
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