Remove obsolete v1 tests (part 1) #553
Conversation
|
Part 2, which will be a separate PR will switch the remaining v1 tests to run as NGSI v2 and change expectations accordingly. It is just easier to define necessary changes without still having unnecessary failing dead v1 tests in the codebase. |
In other similar PRs (telefonicaid/lightweightm2m-iotagent#248 and telefonicaid/iotagent-ul#478), before seens this comment :), I have suggested to use However, please sync this PR with master. Some test has been fixed recently in master branch (PR #550 ) |
|
I can sync this, but the PR should be a two parter - this first step is just deleting the duplicates. The idea being that coverage doesn't drop significantly from this part. I'd prefer to keep part two as a separate PR, as I haven't got time to prioritise all of that work right now, and these two things can be merged separately rather than one great big PR. Trying to keep the PRs small and atomic. |
According to reports in GitHub Actions, coverage would drop 2.4%. It seems to be a bit high... |
This indicates that one of the deleted tests hasn't been duplicated by v2 ... yet 😄 Based on the coverage change I should be able to work out which one that is. |
|
@fgalan - this one is now only down 0.4% - I think that is done. |
Great! The coverage metric would help to check this kind of "gaps" in NGSIv2 test suite :) At the present moment the decrease is 0.4%. Slight above the 0.2%, but fine with me. Do you think the PR is ready to be merged or are you stilling looking for "gaps" that could explain that 0.4% drop? |
Ups... I haven't read your "I think that is done". Ok! |
|
Lines like this would seem to indicate that the remaining reduction in coverage is due to NGSI-v1 specific code (Timestamp Constants and Error handling) so I don't think I can manage better than -0.4% |
|
So adding the additional expectations has reduced the coverage 🤔 Actually it looks like the AMPQ reconnect is sometimes firing, sometimes not. I guess with more tests they likelyhood of a re-connect is higher. |
|
After fixing some other missing V2 tests on #558 this PR need to included new changes in master. Afterwards, it will be ready to be merged |
|
Merged - the flaky AMPQ test timed out though. IThe same test passed the unit test runs, but not the coverage run - maybe give it another kick to see if it is just a timing issue. |
At the second try it worked :) Interestingly, coverage is no ok at GitActions (green mark) although the increase (+) or decrease (-) doesn't appear... Just curiosity I haved tried for get it from the console log of the job but I found it. (Not very important, anyway) |
|
LGTM |
3 participants
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 one is 0.2%
The failure is due to #550 . The coverage can be seen here: https://coveralls.io/github/jason-fox/iotagent-json?branch=master