Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

scripts: ci: check_compliance strips required information #68037

Closed
JordanYates opened this issue Jan 24, 2024 · 2 comments · Fixed by #72592
Closed

scripts: ci: check_compliance strips required information #68037

JordanYates opened this issue Jan 24, 2024 · 2 comments · Fixed by #72592
Assignees
@carlescufi
Labels
area: Continuous Integration bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug

Comments

@JordanYates
Copy link
Collaborator

JordanYates commented Jan 24, 2024 •
edited

Describe the bug

The error parsing regex in check_compliance.py strips out information required to fix the problem for duplicate-code warnings:

zephyr/scripts/ci/check_compliance.py

Line 950 in 8a2a44f

regex = r'^\s*(\S+):(\d+):(\d+):\s*([A-Z]\d{4}):\s*(.*)$'

Logs and console output

For code triggering the similarity checker, check_compliance.py outputs the following:

Running Nits             tests in /Users/user/code/workspace/repo ...
Running Pylint           tests in /Users/user/code/workspace/repo ...
/Users/user/code/workspace/zephyr/scripts/repontrc
pylint --rcfile=/Users/user/code/workspace/zephyr/scripts/ci/pylintrc --load-plugins=argparse-checker scripts/build_release.py scripts/util/add_release_metadata.py
::warning file=scripts/util/add_release_metadata.py,line=1,col=0,title=R0801::scripts/util/add_release_metadata.py:1 Similar lines in 2 files
Running ModulesMaintainers tests in /Users/user/code/workspace/repo ...
Skipping KconfigBasic
Running Checkpatch       tests in /Users/user/code/workspace/repo ...
Running ImageSize        tests in /Users/user/code/workspace/repo ...
1 checks failed
ERROR   : Test Pylint failed:
R0801:Similar lines in 2 files
File:scripts/util/add_release_metadata.py
Line:1
Column:0

Complete results in compliance.xml

and compliance.xml contains:

<?xml version="1.0" encoding="utf-8"?>
<testsuites tests="12" failures="1" errors="0" skipped="0" time="0.0">
	<testsuite name="Compliance" tests="12" errors="0" failures="1" skipped="0" time="0">
		<testcase name="GitDiffCheck" classname="Guidelines"/>
		<testcase name="YAMLLint" classname="Guidelines"/>
		<testcase name="DevicetreeBindings" classname="Guidelines"/>
		<testcase name="Identity" classname="Guidelines"/>
		<testcase name="MaintainersFormat" classname="Guidelines"/>
		<testcase name="Gitlint" classname="Guidelines"/>
		<testcase name="BinaryFiles" classname="Guidelines"/>
		<testcase name="Nits" classname="Guidelines"/>
		<testcase name="Pylint" classname="Guidelines">
			<failure message="scripts/util/add_release_metadata.py:1 Similar lines in 2 files" type="warning">
R0801:Similar lines in 2 files
File:scripts/util/add_release_metadata.py
Line:1
Column:0</failure>
		</testcase>
		<testcase name="ModulesMaintainers" classname="Guidelines"/>
		<testcase name="Checkpatch" classname="Guidelines"/>
		<testcase name="ImageSize" classname="Guidelines"/>
	</testsuite>
</testsuites>

Neither of which are helpful for determining which files have similar code, or where in the file that code is.
It actually points to the wrong location (line 1, instead of line 48).
Running the pylint command manually on the same failing code outputs:

************* Module add_release_metadata
scripts/util/add_release_metadata.py:1:0: R0801: Similar lines in 2 files
==add_release_metadata:[48:66]
==build_release:[107:126]
    boot = edt.label2node['boot_partition']
    app = edt.label2node['slot0_partition']
    scratch = edt.label2node.get('firmware_patch_partition', None)
    partitions = {
        'bootloader': {
            'offset': boot.regs[0].addr,
            'size': boot.regs[0].size,
        },
        'application': {
            'offset': app.regs[0].addr,
            'size': app.regs[0].size,
        },
    }
    if scratch is not None:
        partitions['scratch'] = {'size': scratch.regs[0].size}
    return partitions
@JordanYates JordanYates added bug The issue is a bug, or the PR is fixing a bug area: Continuous Integration labels Jan 24, 2024
@henrikbrixandersen henrikbrixandersen added the priority: low Low impact/importance bug label Jan 30, 2024
@nashif
Copy link
Member

nashif commented Jan 31, 2024

The error parsing regex in check_compliance.py strips out information required to fix the problem for duplicate-code warnings:

zephyr/scripts/ci/check_compliance.py

Line 950 in 8a2a44f

regex = r'^\s*(\S+):(\d+):(\d+):\s*([A-Z]\d{4}):\s*(.*)$'

@carlescufi that was you adding this, can you please take a look?

@nashif nashif assigned carlescufi and unassigned nashif and stephanosio Jan 31, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 1, 2024

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

@github-actions github-actions bot added the Stale label Apr 1, 2024
@JordanYates JordanYates removed the Stale label Apr 1, 2024
carlescufi added a commit to carlescufi/zephyr that referenced this issue May 10, 2024
@carlescufi
scripts: compliance: handle multi-line output from pylint
3b5d8f2 
Switch from plain text to JSON output in the pyling compliance check in
order to handle multi-line messages, which were so far being dropped
by the regex.

Fixes zephyrproject-rtos#68037.

Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
@carlescufi carlescufi mentioned this issue May 10, 2024
Merged
carlescufi added a commit to carlescufi/zephyr that referenced this issue May 10, 2024
@carlescufi
scripts: compliance: handle multi-line output from pylint
54300e6 
Switch from plain text to JSON output in the pylint compliance check in
order to handle multi-line messages, which were so far being dropped
by the regex.

Fixes zephyrproject-rtos#68037.

Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
nashif pushed a commit that referenced this issue May 14, 2024
@carlescufi @nashif
scripts: compliance: handle multi-line output from pylint
ae9096f 
Switch from plain text to JSON output in the pylint compliance check in
order to handle multi-line messages, which were so far being dropped
by the regex.

Fixes #68037.

Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
ycsin pushed a commit to ycsin/zephyr that referenced this issue May 17, 2024
@carlescufi @ycsin
scripts: compliance: handle multi-line output from pylint
b064fe6 
Switch from plain text to JSON output in the pylint compliance check in
order to handle multi-line messages, which were so far being dropped
by the regex.

Fixes zephyrproject-rtos#68037.

Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@carlescufi carlescufi

Labels
area: Continuous Integration bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

scripts: compliance: handle multi-line output from pylint carlescufi/zephyr
6 participants
@nashif @henrikbrixandersen @JordanYates @stephanosio @carlescufi @galak