Feature/unit tests#63
Conversation
…into feature/unit-tests
| const assertNullOrUndefined = Validator.assertNullOrUndefined.bind(Validator); | ||
| const assertNotNull = Validator.assertNotNull.bind(Validator); | ||
|
|
||
| const testNullBase = Validator.testNullBase.bind(Validator, |
There was a problem hiding this comment.
It may be better to just invoke Validator.testNullBase directly where it is used in the case where it only appears once in the file. bind could change to apply if the context is still necessary
| it ('testRpcValues', function (done) { | ||
| let msg = this.msg; | ||
| // Test Values | ||
| const testSupportedDiagModes = msg.getSupportedDiagModes(); |
There was a problem hiding this comment.
It would be easier to check if all these properties were accounted for if they were written in the same order as the set methods in the createMessage function, or if they were more lexically close together.
There was a problem hiding this comment.
I updated the order here to match the param order in the mobile rpc spec https://github.com/smartdevicelink/rpc_spec/blob/version/6_0_0/MOBILE_API.xml . setSystemSoftwareVersion and setHMICapabilities were missing and so those were added to this now as well.
| const assertNull = Validator.assertNull.bind(Validator); | ||
| const assertNullOrUndefined = Validator.assertNullOrUndefined.bind(Validator); | ||
| const assertNotNull = Validator.assertNotNull.bind(Validator); | ||
| const testNullBase = Validator.testNullBase.bind(Validator, |
There was a problem hiding this comment.
A lot of these seem to be unused in here and in other files. The linter should be able to find unused references.
There was a problem hiding this comment.
removed unused variables
| assertNotNull(Test.NOT_NULL, rpcMessage); | ||
| testNullBase(rpcMessage); | ||
|
|
||
| assertNullOrUndefined(Test.NULL, rpcMessage.getSdlMsgVersion()); |
There was a problem hiding this comment.
Should have/use a string that describes that null or undefined are the expected values (ex. Test.NULL_OR_UNDEFINED)
There was a problem hiding this comment.
I removed the the message for these assertion where it was a generic message. The error returned by default is accurate. For this one if there is an error it should say something like "line 116 expected null to not be null or undefined". The default messages were something I copied over from the java suite but they are not really relevant here. The custom error messages like 'Correlation ID should be defined.' are still there.
| const assertNullOrUndefined = Validator.assertNullOrUndefined.bind(Validator); | ||
| const assertNotNull = Validator.assertNotNull.bind(Validator); | ||
|
|
||
| let msg; |
There was a problem hiding this comment.
This looks inconsistent with the other files with how they declare msg from within the tests
There was a problem hiding this comment.
Added BaseStructTests which acts similarly to BaseRpcTests.
|
|
||
| /** | ||
| * Validate an rpcMessage matches the given expectedParameters | ||
| * @param {RpcMessage} rpcMessage - ssageto validate against. |
nickschwab
left a comment
nickschwab
left a comment
There was a problem hiding this comment.
Revisions requested - see inline comments.
3 participants
Fixes #60
This PR is ready for review.
Risk
This PR makes no API changes.
Testing Plan
npm run testscript added which will run tests under the tests/node directory. Right now there are basic examples of request, response, and notification rpcs.Summary
Basic request, response, and notification rpc tests copied over from the java suite sdk and adjusted to work with mocha instead of JUnit.
CLA