Skip to content

Commit 632a2be

Browse files
committedJun 5, 2017
Disable QgsTaskManager::waitForFinished test by default
The test intermittently fails on Travis builds, likely due to the platform's inconsistent availability to multiple threads.
1 parent 176b7ca commit 632a2be

File tree

1 file changed

+13
-3
lines changed

1 file changed

+13
-3
lines changed
 

‎tests/src/core/testqgstaskmanager.cpp

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,12 @@
2222
#include <QObject>
2323
#include "qgstest.h"
2424

25+
// some of these tests have intermittent failure on Travis, probably due
26+
// to inconsistent availability to multiple threads on the platform.
27+
// These tests are disabled unless WITH_FLAKY is 1.
28+
29+
#define WITH_FLAKY 0 //TODO - disable only for Travis?
30+
2531
class TestTask : public QgsTask
2632
{
2733
Q_OBJECT
@@ -194,13 +200,15 @@ class TestQgsTaskManager : public QObject
194200
void task();
195201
void taskResult();
196202
void taskFinished();
197-
#if 0 //flaky
203+
#if WITH_FLAKY
198204
void subTask();
199205
#endif
200206
void addTask();
201207
void taskTerminationBeforeDelete();
202208
void taskId();
209+
#if WITH_FLAKY
203210
void waitForFinished();
211+
#endif
204212
void progressChanged();
205213
void statusChanged();
206214
void allTasksFinished();
@@ -434,7 +442,7 @@ void TestQgsTaskManager::taskFinished()
434442
QCOMPARE( *resultObtained, false );
435443
}
436444

437-
#if 0 //flaky
445+
#if WITH_FLAKY
438446
void TestQgsTaskManager::subTask()
439447
{
440448
QgsTaskManager manager;
@@ -657,6 +665,7 @@ void TestQgsTaskManager::taskId()
657665
delete task3;
658666
}
659667

668+
#if WITH_FLAKY
660669
void TestQgsTaskManager::waitForFinished()
661670
{
662671
QgsTaskManager manager;
@@ -696,6 +705,7 @@ void TestQgsTaskManager::waitForFinished()
696705
QCOMPARE( timeoutTooShortTask->waitForFinished( 20 ), false );
697706
QCOMPARE( timeoutTooShortTask->status(), QgsTask::Running );
698707
}
708+
#endif
699709

700710
void TestQgsTaskManager::progressChanged()
701711
{
@@ -1115,7 +1125,7 @@ void TestQgsTaskManager::managerWithSubTasks()
11151125
QCOMPARE( manager->activeTasks().count(), 1 );
11161126
QVERIFY( manager->activeTasks().contains( parent ) );
11171127
QCOMPARE( spy.count(), 1 );
1118-
#if 0 // flaky
1128+
#if WITH_FLAKY
11191129
//manager should not directly listen to progress reports from subtasks
11201130
//(only parent tasks, which themselves include their subtask progress)
11211131
QCOMPARE( spyProgress.count(), 0 );

5 commit comments

Comments
 (5)

m-kuhn commented on Jun 5, 2017

@m-kuhn
Member

I think there is actually a race condition somewhere, that I couldn't pin down yet. I just hope it's in the test and not the waitForFinished() code.

nyalldawson commented on Jun 5, 2017

@nyalldawson
CollaboratorAuthor

Should I reenable? I just know that many other tests which rely on multiple threads are flaky on Travis. e.g. there's others in the task manager test suite which i've had to disable, and https://github.com/qgis/QGIS/blob/master/tests/src/python/test_qgslocator.py

None of these I can get to fail locally, despite everything I throw at it. Same with the waitForFinished test.

m-kuhn commented on Jun 6, 2017

@m-kuhn
Member

I would only re-enable once additional debug output has been added to investigate the issue.

I don't have any lead yet. But I have a bad feeling with putting them aside as "travis issues" and fear that deadlocks sit waiting somewhere to surface when they are least expected...

nyalldawson commented on Jun 6, 2017

@nyalldawson
CollaboratorAuthor

I don't have any lead yet. But I have a bad feeling with putting them aside as "travis issues" and fear that deadlocks sit waiting somewhere to surface when they are least expected...

Me too! But it was failing quite a lot, and was at the level where we've previously disabled whole tests. I'd rather ifdef out a single test then the whole set of task manager tests....

m-kuhn commented on Jun 6, 2017

@m-kuhn
Member

Did you write a note on the mailing list? :P

Seriously, I was just about to disable it myself

Please sign in to comment.