Skip to content

Conversation

@jason-fox
Copy link
Contributor

  • These tests are duplicated by equivalent v2 tests

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.

@fgalan
Copy link
Member

fgalan commented Apr 13, 2021

Maybe this PR shoud use jason-fox:feature/ngsi-v1 as dependency for iotagent-node-lib in packages.json so we can use it at the same time to check that the new version of the library will work fine with the agent?

In addition, please sync this PR with master. Some test has been fixed recently in master branch (PR #476 )

@fgalan
Copy link
Member

fgalan commented Apr 13, 2021

Similar to telefonicaid/iotagent-json#553 (comment), we should consider if the coverage drop (2.3% this case) is "within acceptable limits".

@jason-fox jason-fox marked this pull request as draft April 13, 2021 11:42
@jason-fox jason-fox marked this pull request as ready for review April 13, 2021 12:43
@jason-fox
Copy link
Contributor Author

This one is now 0.8%, but I can't see any obvious gap. Looking at the coverage report the drop is due to code that needs to be deleted such as this NGSI-v1 constant or AMPQ disconnection which is due to there now only being 1 connection test not two.

I think this is all that can be done for this pass.

@fgalan fgalan requested a review from mapedraza April 13, 2021 16:18
@jason-fox
Copy link
Contributor Author

As can be seen from the equivalent here some of the drop is indeed due to the AMQP connection reducing the number of tests and potentially not needing a disconnect() - this makes a - 0.8% drop more tolerable.

@jason-fox jason-fox marked this pull request as draft May 17, 2021 07:28
@jason-fox jason-fox closed this May 17, 2021
@fgalan
Copy link
Member

fgalan commented May 17, 2021

Just to know... why this PR has been closed? Maybe due to the force push?

@jason-fox
Copy link
Contributor Author

Probably. I've rebased, amended the scope to delete only and re-raised as #487

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.

2 participants