D14364: Test configId in TestSerializer

[Roman Gilg]

romangg added inline comments.

INLINE COMMENTS

> gladhorn wrote in serializertest.cpp:163
> I was hoping the comment above is enough, repeated explanations are not that sensible either in my opinion. I need the constant configId to compare against it later, the point is to confirm that the configId is independent of the order of outputs in the hash for example.

You can leave the comment out if you want to.

My core "complain" is this: In your code the line 163

  QCOMPARE(Serializer::configId(config), configId);

checks, that the config file names stay the same. For that it compares the config id value at the time the test is run to a static one, which you have fixated at an arbitrary point in time (which was when you wrote this change). This test is to make sure, that config id value stays the same over time.

Below you have in line 172 again a comparison. But this time it makes sure that on changes to output ids the config id stays the same. This is not dependent on how the static value was at the time you wrote the patch but only on the value of the current one when the test is run.

For example the current value when the test is run could become different to the fixated one. But the current value should (and could) still be constant over output id changes. Of course this does not influence the result of the test, because it fails already above in this case. But in theory it makes more sense to compare config ids on output id changes in regards to a variable holding the config id value when the test is run instead of comparing it with the fixated one.

REPOSITORY
  R104 KScreen

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

To: gladhorn, #plasma, romangg
Cc: romangg, 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/20180727/0e4d3d16/attachment-0001.html>