-
Notifications
You must be signed in to change notification settings - Fork 45
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 js tests for wiring connections #236
Add js tests for wiring connections #236
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the pull request, as there are conflicts, and address the requested changes.
@@ -26,9 +26,9 @@ | |||
|
|||
"use strict"; | |||
|
|||
// ========================================================================= | |||
// ================================================================================== |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Breaks code style
// CLASS DEFINITION | ||
// ========================================================================= | ||
// ================================================================================== |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Breaks code style
} | ||
|
||
}); | ||
|
||
// ========================================================================= | ||
// ================================================================================== |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Breaks code style
// PRIVATE MEMBERS | ||
// ========================================================================= | ||
// ================================================================================== |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Breaks code style
@@ -44,178 +44,145 @@ | |||
* @param {Wiring} wiringEngine | |||
* [TODO: description] | |||
*/ | |||
ns.Connection = utils.defineClass({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one defineClass
less 👍
|
||
describe("remove()", function () { | ||
|
||
it("should remove after disconnecting endpoints", function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"should disconnect the connection"
}); | ||
}); | ||
|
||
describe("remove()", function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing tests removing a disconnected connection
expect(callback.calls.count()).toEqual(1); | ||
}); | ||
|
||
it("should ignore if one of their endpoints is missing", function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Split into two tests:
"should do nothing if the source is a ghost endpoint"
"should do nothing if the target is a ghost endpoint"
expect(callback.calls.count()).toEqual(1); | ||
}); | ||
|
||
it("should ignore if connection is established", function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"should do nothing if the connection is already established"
var callback = jasmine.createSpy("callback"); | ||
|
||
connection.addEventListener("establish", callback); | ||
connection.establish(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
establish
should support method chaining. Change this line to:
expect(connection.establish()).toBe(connection);
dcc33b8
to
c85df8f
Compare
No description provided.