Skip to content

test/cli/test-other.py: Fix test_showtime_top5_file.#5847

Merged
danmar merged 1 commit into
cppcheck-opensource:mainfrom
Rail-Connected:pullreq13
Jan 6, 2024
Merged

test/cli/test-other.py: Fix test_showtime_top5_file.#5847
danmar merged 1 commit into
cppcheck-opensource:mainfrom
Rail-Connected:pullreq13

Conversation

@mvds00
Copy link
Copy Markdown
Contributor

@mvds00 mvds00 commented Jan 6, 2024

test_showtime_top5_file() assumes that all reported elements, of which the "top 5" are validated, end with the string "- 1 result(s))".

This is clearly not the case when viewing the entire list:

$ cppcheck --showtime=summary --quiet empty.c  |grep "2 result"
valueFlowLifetime(tokenlist, errorLogger, settings): 3.4e-05s (avg. 1.7e-05s - 2 result(s))
valueFlowEnumValue(symboldatabase, settings): 3e-06s (avg. 1.5e-06s - 2 result(s))

As the order of items is non-deterministic, this test makes CI workflows randomly fail.

This patch addresses the issue by adjusting the expected string to the reported item.

test_showtime_top5_file() assumes that all reported elements, of which the "top
5" are validated, end with the string "- 1 result(s))".

This is clearly not the case when viewing the entire list:

```bash
$ cppcheck --showtime=summary --quiet empty.c  |grep "2 result"
valueFlowLifetime(tokenlist, errorLogger, settings): 3.4e-05s (avg. 1.7e-05s - 2 result(s))
valueFlowEnumValue(symboldatabase, settings): 3e-06s (avg. 1.5e-06s - 2 result(s))
```

As the order of items is non-deterministic, this test makes CI workflows
randomly fail.

This patch addresses the issue by adjusting the expected string to the reported
item.
@firewave
Copy link
Copy Markdown
Collaborator

firewave commented Jan 6, 2024

This is a known issue: https://trac.cppcheck.net/ticket/12106.

I did not address it yet since I am not sure what would be actually be expected. The calls should possibly have unique identifiers so you can differentiate them otherwise the output might not be helpful. It's hard to tell as I never have used it to identify performance issues.

@mvds00
Copy link
Copy Markdown
Contributor Author

mvds00 commented Jan 6, 2024

This is a known issue: https://trac.cppcheck.net/ticket/12106.

I did not address it yet since I am not sure what would be actually be expected. The calls should possibly have unique identifiers so you can differentiate them otherwise the output might not be helpful. It's hard to tell as I never have used it to identify performance issues.

Thanks for bringing this to light. I've noticed that there are typically just two valueFlow items that consistently report two results. It would be really helpful to address this, as the current situation can lead to some confusion and random failures in CI workflows, which isn't ideal.

@firewave
Copy link
Copy Markdown
Collaborator

firewave commented Jan 6, 2024

Thanks for bringing this to light. I've noticed that there are typically just two valueFlow items that consistently report two results.

That's because we execute some valueflow steps are run multiple times in one iteration. I was also looking into collecting separate timing information for each iteration. It's been a while though so I can't really remember all the details.

It would be really helpful to address this, as the current situation can lead to some confusion and random failures in CI workflows, which isn't ideal.

Definitely. Fortunately it doesn't happen that often. This would be fine as a workaround until we have decided if we want/need to change it.

@danmar
Copy link
Copy Markdown
Collaborator

danmar commented Jan 6, 2024

I think this solutions is ok for now too..

@danmar danmar merged commit 328f98b into cppcheck-opensource:main Jan 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants