D15520: Upgrade SimpleIpV4AddressValidator and SimpleIpV6AddressValidator

[Pino Toscano]

pino requested changes to this revision.
pino added a comment.
This revision now requires changes to proceed.


  - same notes for `simpleipv6test.cpp` as in `simpleipv4test.cpp`

INLINE COMMENTS

> CMakeLists.txt:11-13
> +add_executable(simpleipv6test ${simpleipv6test_SRCS})
> +add_test(simpleipv6 simpleipv6test)
> +ecm_mark_as_test(simpleipv6test)

just use `ecm_add_test` instead

> CMakeLists.txt:18-19
> +                      Qt5::Widgets
> +                      KF5::ConfigCore 
> +                      KF5::CoreAddons)
> +add_library(simpleipv6test_SRCS)

are these two libraries actually used?

> CMakeLists.txt:26-28
> +add_executable(simpleipv4test ${simpleipv4test_SRCS})
> +add_test(simpleipv4 simpleipv4test)
> +ecm_mark_as_test(simpleipv4test)

ditto

> CMakeLists.txt:33-34
> +                      Qt5::Widgets
> +                      KF5::ConfigCore 
> +                      KF5::CoreAddons)
> +add_library(simpleipv4test_SRCS)

ditto

> simpleipv4test.cpp:23
> +#include <QTest>
> +#include <QDir>
> +

unused

> simpleipv4test.cpp:43-45
> +    vb = new SimpleIpV4AddressValidator(nullptr, SimpleIpV4AddressValidator::AddressStyle::Base);
> +    vc = new SimpleIpV4AddressValidator(nullptr, SimpleIpV4AddressValidator::AddressStyle::WithCidr);
> +    vp = new SimpleIpV4AddressValidator(nullptr, SimpleIpV4AddressValidator::AddressStyle::WithPort);

even if this is a test, these are leaked, "polluting" the results of tools like valgrind; just make them non-pointers class variables

> simpleipv4test.cpp:48
> +
> +void SimpleIpv4Test::baseTest()
> +{

please use a data-driven approach for this file

> simpleipv4test.cpp:53
> +    QString tstr("");
> +    QVERIFY(vb->validate(tstr, pos) == QValidator::Intermediate);
> +

`QVERIFY(foo == bar)` -> `QCOMPARE(foo, bar)`, it applies to all the checks in this file

REPOSITORY
  R116 Plasma Network Management Applet

REVISION DETAIL
  https://phabricator.kde.org/D15520

To: andersonbruce, jgrulich, pino
Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20180917/48b15d06/attachment-0001.html>