Skip to content

Remove obsolete v1 tests (part 1)#487

Merged
mapedraza merged 1 commit intotelefonicaid:masterfrom
jason-fox:ngsi-v1
Jul 14, 2021
Merged

Remove obsolete v1 tests (part 1)#487
mapedraza merged 1 commit intotelefonicaid:masterfrom
jason-fox:ngsi-v1

Conversation

@jason-fox
Copy link
Copy Markdown
Contributor

In order to merge telefonicaid/iotagent-node-lib#995 I want to get the unit-tests running without relying on NGSI-v1. This PR just deletes tests which are already duplicated in the NGSI-v2 test suite. Any drop in coverage should be within acceptable limits

This is the ultralight equivalent of iotagent-json/553. Supersedes #478

@jason-fox
Copy link
Copy Markdown
Contributor Author

I'm not sure what coveralls means here - I can't see any significant coverage difference:

Screenshot 2021-05-17 at 16 21 51

Running locally is also different:

Screenshot 2021-05-17 at 16 24 00

@jason-fox
Copy link
Copy Markdown
Contributor Author

The miss on commonBindings.js line 62 is totally expected - it is NGSI-v1 specific code.

Comment on lines -192 to -214
it('should add a transport to the registered devices', function (done) {
const getDeviceOptions = {
url: 'http://localhost:' + config.iota.server.port + '/iot/devices/MQTT_UNPROVISIONED',
method: 'GET',
headers: {
'fiware-service': 'TestService',
'fiware-servicepath': '/testingPath'
}
};

request(getOptions, function (error, response, body) {
request(getDeviceOptions, function (error, response, body) {
should.not.exist(error);

const parsedBody = JSON.parse(body);

response.statusCode.should.equal(200);
should.exist(parsedBody.transport);
parsedBody.transport.should.equal('HTTP');
done();
});
});
});
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This it section is not present on NGSI-v2 test here

@mapedraza
Copy link
Copy Markdown
Collaborator

mapedraza commented Jun 11, 2021

Im going to summarize all changes needed to have the same coverage (or, at least, the same "scenarios") on V1 and V2 for an effective v1 removal from IoTA Node Lib codebase. Not all of them apply to this specific PR (since still remains some tests on V1 folder)

  1. Fix tests that are not matching on V2

  2. Coppy tests that are NGSI agnostic to V2 folder

    • startup-test.js
    • ultralightMeasures-test.js
    • ultralightCommands-test.js
  3. Implement missing tests in V2

    • configurationsMqtt-test.js

@fgalan
Copy link
Copy Markdown
Member

fgalan commented Jul 13, 2021

Im going to summarize all changes needed to have the same coverage (or, at least, the same "scenarios") on V1 and V2 for an effective v1 removal from IoTA Node Lib codebase. Not all of them apply to this specific PR (since still remains some tests on V1 folder)

PR #499 adds missing NGSIv2 tests.

I think this PR is now ready to be merged. @jason-fox I understand that after this PR an equivalent of PR telefonicaid/iotagent-json#561 ("Switch tests to NGSI-v2") will be done, right?

Copy link
Copy Markdown
Member

@fgalan fgalan left a comment

Choose a reason for hiding this comment

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

LGTM

Passing the ball to @mapedraza for final LGTM and merge

Copy link
Copy Markdown
Collaborator

@mapedraza mapedraza left a comment

Choose a reason for hiding this comment

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

LGTM too

@mapedraza mapedraza merged commit ec7aeed into telefonicaid:master Jul 14, 2021
@fgalan
Copy link
Copy Markdown
Member

fgalan commented Jul 14, 2021

I think this PR is now ready to be merged. @jason-fox I understand that after this PR an equivalent of PR telefonicaid/iotagent-json#561 ("Switch tests to NGSI-v2") will be done, right?

I have discussing this with @mapedraza and it seems that the ""Switch test to NGSI-v2" PR is not needed in this case. However, it seems some NGSI-v1 tests are pending from removal. PR #505 should fix that.

@fgalan fgalan mentioned this pull request Jul 14, 2021
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.

3 participants