On 1/12/21 10:09 PM, Mark Michelson wrote:
> This is a new version of a patch series that formerly introduced a new unit
> test framework into OVN.
> 
> In this version, the new unit test framework has been removed. It
> instrucmented existing code rather than living separately, and that was
> not appreciated by those who were reviewing the changes. In this new
> version, I am taking the IPAM refactoring and the tests I added for
> IPAM, and I am reworking them to just use ovstest instead.
> 
> Some additional unit test scaffolding may at some point be required if,
> for instance, we wish to mock interaction with a database or with OVS.
> That, however, is not necessary for the tests being added here.
> 
> v6 -> v7:
>   * Removed unnecessary ENABLE_UNIT_TESTS checks in ovn-ipam.at
> 
> v5 -> v6:
> Patch 1:
>   * Added "id" field to ipam_info struct so that some sort of contextual
>     information can be present in log messages. In our case, we always
>       pass logical switch UUID here. This id is printed in several log
>       messages.
>   * Made the ipv4 and ipv6 initialization functions static. This makes
>     it more difficult to misuse the IPAM API by partially initializing
>       an ipam_info struct.
>   * Moved log messages that mention specific reasons for IPAM failures
>     into ipam.c
>   * Clarified a XXX comment with more details.
> 
> Patch 2:
>   * Changed IPv4 and IPv6 tests to use the general ipam_info
>     initialization function since the ipv4 and ipv6 initialization
>       functions are no longer public.
>   * Changed test file name to ovn-ipam.at
>   * Fixed automake.mk typo
> 
> v4 -> v5:
> * Moved some of the changes in patch 2 to patch 1 so that it would
>   compile properly.
> 
> Mark Michelson (3):
>   northd: refactor and split some IPAM functions
>   Add ipam unit tests
>   Disable logging to the console from ovstest.
> 
>  northd/automake.mk  |   5 +-
>  northd/ipam.c       | 335 ++++++++++++++++++++++++++++++++++++++++++++
>  northd/ipam.h       |  39 ++++++
>  northd/ovn-northd.c | 291 ++++----------------------------------
>  northd/test-ipam.c  | 146 +++++++++++++++++++
>  tests/automake.mk   |   8 +-
>  tests/ovn-ipam.at   | 237 +++++++++++++++++++++++++++++++
>  tests/ovstest.c     |  17 +++
>  tests/testsuite.at  |   1 +
>  9 files changed, 810 insertions(+), 269 deletions(-)
>  create mode 100644 northd/ipam.c
>  create mode 100644 northd/ipam.h
>  create mode 100644 northd/test-ipam.c
>  create mode 100644 tests/ovn-ipam.at
> 

Hi Mark,

I replied by accident with (minor) comments and acks for the v6 patch set:
https://mail.openvswitch.org/pipermail/ovs-dev/2021-January/379458.html
https://mail.openvswitch.org/pipermail/ovs-dev/2021-January/379459.html
https://mail.openvswitch.org/pipermail/ovs-dev/2021-January/379460.html

I actually intended to reply to this patch set (v7) instead so please
consider my previous comments for this version.

Regards,
Dumitru

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to