Remove messages of already running CTests for UnitTests
ClosedPublic

Authored by croick on Jul 9 2017, 11:12 PM.

Details

Summary
  • starting unit tests for a project or simply "Run All Tests" could start showing up messages of already running instances of CTest
  • after killing or not killing those, KDevelop could finally crash

BUG: 377639

Fixes:

  • use job name in NativeAppJob depending on executable to be able to distinguish between different tests
  • after the message box, re-read the list of running jobs
  • actual bugfix: only emit the result once for failing tests
Test Plan
  • run kdevelop/kdevelop unit tests at once
  • see what happens after tests are failing

Diff Detail

Repository
R33 KDevPlatform
Branch
ctest
Lint
No Linters Available
Unit
No Unit Test Coverage
croick created this revision.Jul 9 2017, 11:12 PM
Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptJul 9 2017, 11:12 PM
apol added a comment.Jul 10 2017, 10:06 AM

Other than that, LGTM. I'm not sure why you need to go through the job list again, but it's not a big penalty so if that's what makes it stable, let's go for it.

plugins/execute/nativeappjob.cpp
51 ↗(On Diff #16416)

Either do splitRef or section.

croick updated this revision to Diff 16473.Jul 10 2017, 8:28 PM
  • use section instead of split for path separation
croick marked an inline comment as done.Jul 10 2017, 8:37 PM
In D6593#123646, @apol wrote:

Other than that, LGTM. I'm not sure why you need to go through the job list again, but it's not a big penalty so if that's what makes it stable, let's go for it.

I experienced a crash in operator==() of job->m_cfgname == m_cfgname. Theoretically, the same job could be started three times (which was not unusual while all tests were still called "CTest"), so it was possible to kill all jobs in the third instance, while the second is not handled at first. When the second finally accessed job->m_cfgname for a job that is deleted already, the program crashed.

apol accepted this revision.Jul 11 2017, 12:46 AM
This revision is now accepted and ready to land.Jul 11 2017, 12:46 AM
This revision was automatically updated to reflect the committed changes.