Fixed #12771 (Progress value is not increased) #7956

danmar · 2025-11-11T19:02:23Z

No description provided.

Copilot

Pull Request Overview

This PR fixes issue #12771 where the progress value was not being increased properly. When processing files using FileSettings (e.g., from compilation databases), the total file size is often 0, causing the progress percentage to always show 0%. The fix switches to using file count instead of file size for progress calculation when FileSettings are used or when total file size is 0.

Modified progress reporting logic in ThreadExecutor and ProcessExecutor to use file counts instead of file sizes when appropriate
Updated existing test to handle both percentage-based (0%, 25%, etc.) and file-count-based (0%) progress messages
Added new parametrized test to verify progress reporting works correctly with different executor types and job counts

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
cli/threadexecutor.cpp	Modified to use file count for progress when total file size is 0
cli/processexecutor.cpp	Modified to use file count for progress when using FileSettings
test/cli/other_test.py	Updated test to accept both 0% and percentage-based progress messages
test/cli/more-projects_test.py	Added new test to verify progress reporting with compilation database

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

cli/threadexecutor.cpp

cli/processexecutor.cpp

test/cli/other_test.py

firewave · 2025-11-12T11:43:55Z

test/cli/other_test.py

+        try:
+            if '{}/4 files checked 0% done'.format(i) in lines:
+                lines.remove('{}/4 files checked 0% done'.format(i))
+            else:
+                lines.remove('{}/4 files checked {}% done'.format(i, i * 25))
+        except ValueError:
+            assert False, f"Expected progress message for file {i}/4 not found in output. Checked for '{{}}/4 files checked 0% done' and '{{}}/4 files checked {{}}% done'. Output lines: {lines}"


This is awkward. One way to improve it would be to put the expected message in a variable and use that in the checks so it is not duplicated.

And we should be expecting a specific output and not multiple ones. That dilutes the test.

And the message in the except is confusing.

And we should be expecting a specific output and not multiple ones. That dilutes the test.

As far as I know it differs depending on in which order the files are finished. And the order will change because we use -j2. A test that runs fine locally does not run fine in CI.

As far as I know it differs depending on in which order the files are finished

the status is not reported in this way for markup files.

the status is not reported in this way for markup files.

Probably https://trac.cppcheck.net/ticket/12167

As far as I know it differs depending on in which order the files are finished. And the order will change because we use -j2. A test that runs fine locally does not run fine in CI.

Then we should split the test as long as they differentiate as allowing multiple results would not detect regressions or fixes.

firewave · 2025-11-12T11:48:58Z

A side note regarding progress which is outside of this scope.

We had a discussion about using the file size to calculate the percentage. I was arguing that we should not be doing it since it seems arbitrary and confusing as the size might not reflect actual runtime and might be disproportionate to other files. While looking into stuff related to preprocessor configurations I realized that would be even more arbitrary because if a file which is very small but has lots of extracted configurations that is considered for the calculation. So the size might only makes sense if you have a single fixed configuration per file - and even then that configuration might reduce the file to nothing. And adding logic which takes the actual size after the preprocessing has been applied would be madness.

Co-authored-by: Copilot <[email protected]>

danmar · 2025-11-14T12:44:54Z

@firewave now some CIs fail like this:

  -     '1/4 files checked 0% done',
  ?                        ^
  +     '1/4 files checked 25% done',
  ?                        ^^
  -     '2/4 files checked 0% done',
  +     '2/4 files checked 50% done',
  ?                        +
  -     '3/4 files checked 0% done',
  ?                        ^
  +     '3/4 files checked 75% done',
  ?                        ^^
  -     '4/4 files checked 0% done',
  +     '4/4 files checked 100% done',
  ?                        ++

Other CIs succeed.

The test is successful locally; I get "0%" on my computer. (linux). I am not sure right now why there are different results locally and in CI but maybe platform makes a difference.

danmar · 2025-11-14T12:56:51Z

The test is successful locally; I get "0%" on my computer. (linux). I am not sure right now why there are different results locally and in CI but maybe platform makes a difference.

I am not sure why it says "0%" on my computer I will try to dig into that soon..

firewave · 2025-11-14T16:26:04Z

So splitting it actually found another bug - nice.

I never thought about the 0% when I wrote those other tests. That should have resulted in a ticket.

And I think we should still file one as it is a different issue then this PR was aiming to address. Otherwise I approve.

sonarqubecloud · 2025-11-14T16:37:47Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

danmar force-pushed the fix-12771 branch from 01ee47f to 3861d0b Compare November 11, 2025 21:10

danmar requested review from Copilot and firewave November 12, 2025 07:07

Copilot started reviewing on behalf of danmar November 12, 2025 07:08 View session

Copilot finished reviewing on behalf of danmar November 12, 2025 07:09

Copilot AI reviewed Nov 12, 2025

View reviewed changes

cli/threadexecutor.cpp Outdated Show resolved Hide resolved

cli/processexecutor.cpp Outdated Show resolved Hide resolved

test/cli/other_test.py Outdated Show resolved Hide resolved

firewave reviewed Nov 12, 2025

View reviewed changes

danmar and others added 5 commits November 13, 2025 15:19

Fixed #12771 (Progress value is not increased)

811328e

scriptcheck

a2fcce0

Update cli/processexecutor.cpp

4784830

Co-authored-by: Copilot <[email protected]>

Update test/cli/other_test.py

48ad09a

Co-authored-by: Copilot <[email protected]>

refactor test

796766f

danmar force-pushed the fix-12771 branch from 4370669 to 796766f Compare November 13, 2025 14:26

danmar and others added 5 commits November 13, 2025 16:29

fix

bc3c159

import re

eb3c6cd

only qml files

090d573

test is about testing progress messages

9833ff9

test

34d53fd

test

ab71c6b

danmar marked this pull request as draft November 14, 2025 14:57

danmar added 2 commits November 14, 2025 16:04

test

05f1b0d

moreprojects

d2952c2

firewave previously approved these changes Nov 14, 2025

View reviewed changes

cleanup

11b6178

danmar dismissed firewave’s stale review via 11b6178 November 14, 2025 16:37

danmar marked this pull request as ready for review November 14, 2025 17:51

danmar merged commit f7a3761 into danmar:main Nov 14, 2025
55 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fixed #12771 (Progress value is not increased) #7956

Fixed #12771 (Progress value is not increased) #7956

danmar commented Nov 11, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

firewave Nov 12, 2025

Uh oh!

danmar Nov 13, 2025

Uh oh!

danmar Nov 13, 2025

Uh oh!

firewave Nov 13, 2025

Uh oh!

firewave commented Nov 12, 2025

Uh oh!

danmar commented Nov 14, 2025

Uh oh!

danmar commented Nov 14, 2025

Uh oh!

firewave commented Nov 14, 2025

Uh oh!

sonarqubecloud bot commented Nov 14, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Fixed #12771 (Progress value is not increased) #7956

Fixed #12771 (Progress value is not increased) #7956

Conversation

danmar commented Nov 11, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

firewave Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

danmar Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

danmar Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

firewave Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

firewave commented Nov 12, 2025

Uh oh!

danmar commented Nov 14, 2025

Uh oh!

danmar commented Nov 14, 2025

Uh oh!

firewave commented Nov 14, 2025

Uh oh!

sonarqubecloud bot commented Nov 14, 2025

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants