-
Notifications
You must be signed in to change notification settings - Fork 30
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
Task/travis ci #105
Task/travis ci #105
Conversation
dcalvoalonso
commented
Dec 19, 2017
- It modifies the agent to pass unit tests
- It enables Travis CI
.travis.yml
Outdated
|
||
install: | ||
- npm install | ||
|
||
before_install: | ||
- docker pull fiware/orion:0.26 |
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.
Orion 0.26 is pretty old... I'll sugest to use fiware:orion:latest. This may casuse some ocasionally breaking from time to time (as Orion may evolve) but I think is a good idea to be sure we keep backward compatibility as Orion evolves.
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.
Fixed in 0de8f25
@@ -144,7 +144,8 @@ function selectRealName(extractedName, device, callback) { | |||
* @param {String} type Type of the entity. | |||
* @param {Array} attributes List of attributes of the entity to be updated (including type and value). | |||
*/ | |||
function ngsiUpdateHandler(id, type, attributes, callback) { | |||
function ngsiUpdateHandler(id, type, service, subservice, attributes, callback) { |
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.
This PR incluces changes in program code, along with tests. Anything worth to mention in CHANGES_NEXT_RELEASE (i.e. some bug being fixed in the process of fixing the tests) or only trivial changes?
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.
Fixed in 0de8f25
test/unit/commands-test.js
Outdated
@@ -42,19 +42,35 @@ var config = require('./testConfig'), | |||
ngsiClient = ngsiTestUtils.create( | |||
config.ngsi.contextBroker.host, | |||
config.ngsi.contextBroker.port, | |||
'smartGondor', | |||
'/gardens' | |||
'dumbmordor', |
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.
Not sure about this change... In my opinion it should be just
smartGondor -> smartgondor
Why to change to something completely different (dumbmordor)? Why does subservice needs also to be changed?
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.
Fixed in some recent commit?
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.
Fixed in babe984
test/unit/testConfig.js
Outdated
subservice: '/gardens', | ||
providerUrl: 'http://192.168.56.1:4041/NGSI10', | ||
providerUrl: 'http://localhost:4041/NGSI10', |
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.
Does the test work with /v1 instead of /NGSI10? Each time /NGSI10 is used, a kitten dies ;)
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.
Fixed in 0de8f25
|
||
install: | ||
- npm install | ||
|
||
before_install: | ||
- docker pull fiware/orion:latest |
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.
Looking this in detail, .travis.yml file in the two other main IOTAs (UL and JSON) they aren't deploying Orion in before_install:
https://github.com/telefonicaid/iotagent-json/blob/master/.travis.yml#L21
https://github.com/telefonicaid/iotagent-ul/blob/master/.travis.yml#L21
So, why in the case of IOTA LWM2M is different?
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.
The reason is that tests for IOTA LWM2M are not really unit tests since the responses from the Orion CB are not mocked. Instead, an instance of the CB must be deployed and the tests are executed against it.
I guess that we should open a new issue in order to mock CB in the unit tests of IOTA LWM2M.
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.
Issue created at #106. NTC with regards this PR.
CHANGES_NEXT_RELEASE
Outdated
- FEATURE update node version to 4.8.4 | ||
- Unit tests are executed as part of Travis CI. | ||
- npm-shrinkwrap file is updated (#99 and #100) |
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.
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.
Yes, that's correct. Since the npm-shrinkwrap was not updated, actually still the old version of https://github.com/telefonicaid/iotagent-node-lib was used causing these bugs.
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.
- Fixing issues in the provisioning API (Compose device name when not found using device provisioning #99 and Code debug #100) as result of npm-shrinkwrap file update.
We typically don't mention updates of npm-shrinkwrap file in CNR, but this case I think is a good idea, given doing so is fixing issues.
(I understand that #99 and #100 are about bugs in the provisioning API, otherwise my suggestion would be wrong. Please, check)
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.
Fixed in 9ccd486
Once PR telefonicaid/iotagent-node-lib#571 has been merged into library master, this PR should pass travis. Is this correct? |
You are right, in fact they are being passed for Node 6 but no for Node 4. Really strange.... |
LGTM |
Since a real Orion instance is being used, we needed to give some delays to wait for the answers. |
LGTM |