tests(mimetreeparser): Tests that are have no depdenency against MessageViewer should move to MimeTreeParser.
ClosedPublic

Authored by knauss on Sep 9 2019, 8:32 PM.

Details

Diff Detail

Repository
R94 PIM: Message Library
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
knauss created this revision.Sep 9 2019, 8:32 PM
Restricted Application added a project: KDE PIM. · View Herald TranscriptSep 9 2019, 8:32 PM
Restricted Application added a subscriber: kde-pim. · View Herald Transcript
knauss requested review of this revision.Sep 9 2019, 8:32 PM
dvratil requested changes to this revision.Sep 10 2019, 7:43 AM
dvratil added inline comments.
messageviewer/src/messagepartthemes/default/autotests/objecttreeparsertest.cpp
39

Why not move this as well? It does not seem to use anything that is not in the moved tests. Where/how do you draw the line? :)

This revision now requires changes to proceed.Sep 10 2019, 7:43 AM
romangg added a subscriber: romangg.EditedSep 10 2019, 8:48 AM

Thanks for trying out the unified git commit message guideline. :)

Some small notes about the message header: The type should be just test without the s. The subject itself should be start in lower case without period in the end and in imperative, present tense. By that it can also be condensed in size and additional information put into the message:

test(mimetreeparser): move several tests to MimeTreeParser

Tests that have no dependency against MessageViewer should be moved to
MimeTreeParser. That's simply a move of tests to the right place.

More information at https://github.com/angular/angular/blob/3cf2005a93/CONTRIBUTING.md#-commit-message-guidelines

knauss added inline comments.Sep 10 2019, 9:25 AM
messageviewer/src/messagepartthemes/default/autotests/objecttreeparsertest.cpp
39

The line is the tests that need rendering and not only the parsing. That is seen, if the output of testWriter is used see line 58/59:

QVERIFY(testWriter.data().contains([...]))

it is even more obvious with D23807, where we explicitly trigger the rendering step. As you see all reminding tests, trigger rendering. And the rendering is part of MessageViewer, so we can't move those tests to MimeTreeParser.

dvratil accepted this revision.Sep 10 2019, 1:03 PM

I see. Please adjust the commit message as Roman suggested and ship it.

This revision is now accepted and ready to land.Sep 10 2019, 1:03 PM
This revision was automatically updated to reflect the committed changes.

I see. Please adjust the commit message as Roman suggested and ship it.

argh I forgotten to update the commit message, sorry for that.