search

autowarefoundation / autoware-github-actions

Apache License 2.0
17 stars 24 forks source link

fix(clang-tidy): make clang-tidy workflow fail only when it reports errors #306

Closed veqcc closed 4 months ago

veqcc commented 4 months ago

Description

Related PR:

The current clang-tidy fails when there exists a fixes.yaml. It leads to the CI failure even when clang-tidy reports only warnings. I have changed it so that the CI fails only when it reports errors.

@xmfcx The way I used whether clang-tidy reports an errors is the following:

if [ -n "$(grep ": error:" /tmp/clang-tidy-result/report.txt)" ];

I'm not sure there is a better way, so please review it carefully. Thank you in advance!

Tests performed

Not applicable.

Effects on system behavior

Not applicable.

Interface changes

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

Post-review checklist for the PR author

The PR author must check the checkboxes below before merging.

After all checkboxes are checked, anyone who has write access can merge the PR.

veqcc commented 4 months ago

@xmfcx Do you have any idea how to test this change before being merged?

xmfcx commented 4 months ago

I'll create a branch and point here in universe to test :+1:

xmfcx commented 4 months ago

Could you make it so that it will still export the fixes with the -export-fixes /tmp/clang-tidy-result/fixes.yaml command and upload these artifacts as usual?

It can still check for the existence of your new report file.

The fixes file can be used to apply fixes and provides a structured output.

xmfcx commented 4 months ago

https://github.com/autowarefoundation/autoware.universe/actions/runs/9697046266/job/26761095747?pr=7729#step:9:438

Here you can download the artifact.

Maybe we could use the fixes file and search for Level: Warning or error? to look for errors.

veqcc commented 4 months ago

Maybe we could use the fixes file and search for Level: Warning or error? to look for errors.

I'm not sure so need some investigations how the followings are denoted in yaml file:

veqcc commented 4 months ago

Maybe we could use the fixes file and search for Level: Warning or error? to look for errors.

Quick investigation revealed that the warnings with warnings-as-errors also have Level: Warning in yaml file. So we cannot use fixes.yaml for deciding exit code :sob:

veqcc commented 4 months ago

@xmfcx

Could you make it so that it will still export the fixes with the -export-fixes /tmp/clang-tidy-result/fixes.yaml command and upload these artifacts as usual? It can still check for the existence of your new report file. The fixes file can be used to apply fixes and provides a structured output.

done in https://github.com/autowarefoundation/autoware-github-actions/pull/306/commits/90a0f2abba4a7a278f0e4bbda9cce215530eaa51