<table><tr><td style="">tobiasdeiminger added inline comments.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D13203">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D13203#inline-70382">View Inline</a><span style="color: #4b4d51; font-weight: bold;">parttest.cpp:117</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; ">        <span style="color: #aa4000">void</span> <span style="color: #004012">testAnnotWindow</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #aa2211"><<<<<<<</span> <span class="n">HEAD</span>
</div><div style="padding: 0 8px; margin: 0 4px; ">        <span style="color: #aa4000">void</span> <span class="n">testAdditionalActionTriggers</span><span class="p">();</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Please resolve local merge conflict before submitting the diff.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D13203#inline-70383">View Inline</a><span style="color: #4b4d51; font-weight: bold;">parttest.cpp:1511</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #aa2211"><<<<<<<</span> <span class="n">HEAD</span>
</div><div style="padding: 0 8px; margin: 0 4px; "><span style="color: #74777d">// Helper for testAdditionalActionTriggers</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Please resolve local merge conflict before submitting the diff.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D13203#inline-70385">View Inline</a><span style="color: #4b4d51; font-weight: bold;">parttest.cpp:1563</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">  <span class="n">typewriterButton</span><span style="color: #aa2211">-></span><span class="n">click</span><span class="p">();</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">From here on the code belongs logically to the next test case, is it? In a mixed purpose test like class PartTest, imho test methods should work independent from one another. I.e., in testDialogClosed don't assume that you've opened dialog in testTypewriterAnnotTool. Iow tests should be idempotent. If you want to retain state across several verifications, do everything in one method or create a new dedicated class PartTestAnnotations where it's more obvious to couple test methods tightly.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D13203#inline-70395">View Inline</a><span style="color: #4b4d51; font-weight: bold;">parttest.cpp:1565</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">  <span class="n">m_timer</span><span style="color: #aa2211">-></span><span class="n">start</span><span class="p">(</span><span style="color: #601200">1000</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Using CloseDialogHelper would be more RAII</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D13203#inline-70391">View Inline</a><span style="color: #4b4d51; font-weight: bold;">parttest.cpp:1582</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">  <span class="p">}</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">  <span class="n">QVERIFY</span><span class="p">(</span><span class="n">m_clicked</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="p">}</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">The QInputDialog is pure Qt functionality, just closing it is not that interesting in a typewriter test case. The more interesting thing is to verify if annotation is created correctly after closing.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R223 Okular</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D13203">https://phabricator.kde.org/D13203</a></div></div><br /><div><strong>To: </strong>dileepsankhla, tobiasdeiminger<br /><strong>Cc: </strong>ltoscano, ngraham, tobiasdeiminger, aacid, okular-devel<br /></div>