tobiasdeiminger added inline comments.

INLINE COMMENTS

> dileepsankhla wrote in parttest.cpp:1582
> First of all, in CloseDialogHelper, `m_clicked` is always false as 
> `m_part->widget()->findChild<QDialog*>();` returns NULL for the QInputDialog 
> being the activeModalWidget. So I didn't use it.
> 
> Secondly, I agree that the low tests should be independent but I'm using 
> `QTimer` technique in `testTypewriterAnnotTool` to grab the pointer to the 
> active QInputDialog widget for which I need to connect the SIGNAL timeout() 
> to the SLOT testDialogClosed which eventually becomes another test method. So 
> I can't do it in only a single method instead need two different methods in 
> which one of them becomes the slot of QTimer implementation. In this case. do 
> you think I should do the same in the dedicated class like PartTestAnnotation?
> 
> Thirdly, to verify if the annotation is created correctly, what parameter(s) 
> should I check?

> m_part->widget()->findChild<QDialog*>(); returns NULL for the QInputDialog

That's because QInputDialog is not a child of the part, but a "window" with no 
parent. You can slightly modify CloseDialogHelper and it will work:

  void closeDialog()
  {
      //QDialog *dialog = m_part->widget()->findChild<QInputDialog*>();
      QWidget * dialog = qApp->activeModalWidget();

Afaikt this is also compatible with the remaining locations where 
CloseDialogHelper is used. If you apply the change, CloseDialogHelper::m_part 
is no longer required and can be removed. Construct CloseDialogHelper with 
QDialogButtonBox::Cancel or QDialogButtonBox::Ok, to click either the Ok or the 
Cancel button.

> So I can't do it in only a single method instead need two different methods 
> in which one of them becomes the slot of QTimer implementation.

The problem goes away if you use CloseDialogHelper.

> Thirdly, to verify if the annotation is created correctly, what parameter(s) 
> should I check?

A perfect UI test would be to check for the painted annotation. As you deferred 
it because it's too difficult, how about checking 
`part->m_document->page(0)->annotations()` for the expected typewriter 
annotation?

REPOSITORY
  R223 Okular

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

To: dileepsankhla, tobiasdeiminger
Cc: ltoscano, ngraham, tobiasdeiminger, aacid, okular-devel

Reply via email to