D18567: Make testActiveDocumentsGetBestPriority() clean up after itself

Milian Wolff noreply at phabricator.kde.org
Fri Feb 15 12:14:27 GMT 2019


mwolff added a comment.


  In D18567#411767 <https://phabricator.kde.org/D18567#411767>, @thomassc wrote:
  
  > Regarding waiting for the background parser on `TestFile` destruction, I didn't find a way to query whether a parse job is running or to wait for it, if it was started externally, using existing code (but I'm also not familiar with the codebase). As an alternative, one can do `Q_ASSERT(ICore::self()->languageController()->backgroundParser()->isIdle());` in the test cleanup function to ensure that the tests don't leave the background parser running. This might also be a good idea to ensure that the tests don't influence each other in that way.
  
  
  One idea: In `~TestFile` do these things:
  
  a) close and discard the document, if it's opened currently
  b) `BackgroundParser::removeDocument`
  c) if `BackgroundParser::parseJobForDocument` returns non-null, await the `parseJobFinished` signal for that url with some timeout. You can do that e.g. via:
  
    if (backgroundParser->parseJobForDocument(url))
    {
        bool finished = false;
        connect(backgroundParser, &BackgroundParser::parseJobFinished, this, [this, &finished](const IndexedString& finishedUrl) {
            if (finishedUrl == url)
                finished = true;
        };
        QTRY_VERIFY(finished);
    }
  
  
  
  > Regarding document handling, I'm not sure whether it's worth adding a specific test class for the few cases here. I found that in test_buddies.cpp, the test cleanup function does `m_documentController->closeAllDocuments();`. I tried this for the duchain test, but this caused a crash when this actually tried to close the open documents, which I wasn't motivated to debug. Alternatively, one can do `Q_ASSERT(ICore::self()->documentController()->openDocuments().isEmpty());` similarly to the case above to ensure that the tests clean up after themselves. Would you be fine with that?
  
  Right, so we could just do that in `~TestFile` maybe? Anyhow, ignore this for now if it's too much trouble.

REPOSITORY
  R32 KDevelop

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

To: thomassc, #kdevelop, mwolff
Cc: mwolff, kdevelop-devel, gennad, glebaccon, antismap, iodelay, alexeymin, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20190215/89d3490e/attachment.html>


More information about the KDevelop-devel mailing list