Finishes PR #594 #763
Finishes PR #594 #763
Conversation
add HTTPS tests for NGSIv2
|
I provided LGTM on 6d554eb in the old PR (see #594 (comment)) so I'll do now a review just of the three extra commits. |
CHANGES_NEXT_RELEASE
Outdated
| Fix: add ?type parameter in CB request updates to avoid potential entity ambiguities (#733) | ||
| Fix: incomplete HTTPS support for NGSI subscriptions (#593) |
There was a problem hiding this comment.
Original PR (#594) used "NGSIv1" instead of "NGSI". Does the change means that at the end this PR is covering subscriptions both in NGSIv1 and NGSIv2?
(Looking to parent issue #593 (comment) is seems that only NGSIv1 is covered, but maybe that comment is wrong)
There was a problem hiding this comment.
As you can see in the new test I added with this PR https://github.com/dcalvoalonso/iotagent-node-lib/blob/bug/uncomplete-https-support-subscriptions/test/unit/ngsiv2/general/https-support-test.js#L268. registrations are already implemented in NGSIv2.
Probably you have also noticed that this PR only adds some unit tests. The changes the original PR(#594) had for https://github.com/telefonicaid/iotagent-node-lib/blob/master/lib/services/ngsi/subscriptionService.js were integrated by @AlvaroVega in ca8f850 and covered also NGSIv2.
There was a problem hiding this comment.
Thanks for the clarification!
NTC
There was a problem hiding this comment.
LGTM. Thanks!
There is a small conflict in CHANGES_NEXT_RELEASE that needs to be fixed before merging. In the meanwhile, we can wait for some days before merging, just in case @chicco785 has some final feedback to provide, as author of the initial contribution.
…e-lib into bug/uncomplete-https-support-subscriptions
|
Conflicts fixed with 8c0aabb |
|
Some days after, I think is a good moment to merge. |
This PR finishes some very minor comments of #594.
LGTM was obtained in #594 (comment).
The only pending aspect was to split NGSIv2 tests in a separate file to be consistent with the overall approach: #594 (comment)
Moreover, this PR is rebased and changes of the master branch are merged.