Skip to content

make: run static_tests.sh directly with static-tests target #15667

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
Merged
merged 3 commits into from
Jan 5, 2021
Merged

make: run static_tests.sh directly with static-tests target #15667

merged 3 commits into from
Jan 5, 2021

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Dec 17, 2020

Contribution description

This cleans up the make static-tests target to call dist/tools/ci/static_tests.sh directly instead of first calling dist/tools/static-tests.sh which calls ./dist/tools/ci/build_and_test.sh which then calls dist/tools/ci/static_tests.sh.

The only point in doing it that way, is I believe, that ther is an (incomplete) dependency check in dist/tools/static-tests.sh. That dependency check I improved and completed and moved it to dist/tools/ci/static_tests.sh and adapted it for the look and feel of that script.

Testing procedure

Run

make static-tests

(also done by the static-tests GitHub action). If you have any uninstalled dependency for those tests, the script will report that.

Issues/PRs references

None

@miri64 miri64 requested review from aabadie and fjmolinas December 17, 2020 14:14
@miri64 miri64 force-pushed the make/cleanup/static-tests branch from 586f9d6 to 3c3a05d Compare December 17, 2020 14:17
@miri64
Copy link
Member Author

miri64 commented Dec 17, 2020

Mhhh... uncrustify seems to be not installed in the RIOT docker :D

@miri64 miri64 added Area: build system Area: Build system Area: CI Area: Continuous Integration of RIOT components State: waiting for CI update State: The PR requires an Update to CI to be performed first Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation labels Dec 17, 2020
@miri64 miri64 mentioned this pull request Dec 17, 2020
Merged
@miri64 miri64 force-pushed the make/cleanup/static-tests branch from 3c3a05d to b079797 Compare December 17, 2020 15:49
@miri64 miri64 mentioned this pull request Dec 18, 2020
Merged
@miri64 miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs and removed State: waiting for CI update State: The PR requires an Update to CI to be performed first CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Dec 19, 2020
@jia200x
Copy link
Member

jia200x commented Jan 4, 2021

hmmm I noticed the License check runs twice. Is it expected?

@miri64
Copy link
Member Author

miri64 commented Jan 5, 2021 •
edited
Loading

hmmm I noticed the License check runs twice. Is it expected?

Yes, they use two different diff filters. One for modified and renamed files, one for added and copied files. The first generates only a warning (the author of the change might not have introduced that license), the second generates an error, making the static tests fail.

@jia200x
Copy link
Member

jia200x commented Jan 5, 2021

then, everything runs as expected :). Tried both in my laptop and Docker

@jia200x jia200x added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jan 5, 2021
@jia200x
Copy link
Member

jia200x commented Jan 5, 2021

could you remove the bogus commit?

jia200x
jia200x approved these changes Jan 5, 2021
View reviewed changes
Copy link
Member

@jia200x jia200x left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK. Please remove the bogus commit and let's wait for the CI

@miri64
Copy link
Member Author

miri64 commented Jan 5, 2021

Please remove the bogus commit and let's wait for the CI

Done.

@miri64 miri64 force-pushed the make/cleanup/static-tests branch from b079797 to 9066ab2 Compare January 5, 2021 14:30
@miri64 miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 5, 2021
@miri64
Copy link
Member Author

miri64 commented Jan 5, 2021

Fixed an unrelated spelling error.... no idea why that file is even checked...

@miri64 miri64 merged commit 4bcf908 into RIOT-OS:master Jan 5, 2021
@miri64 miri64 deleted the make/cleanup/static-tests branch January 5, 2021 15:41
@crasbe crasbe mentioned this pull request Apr 4, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jia200x jia200x jia200x approved these changes

@aabadie aabadie Awaiting requested review from aabadie

@fjmolinas fjmolinas Awaiting requested review from fjmolinas

Assignees
No one assigned
Labels
Area: build system Area: Build system Area: CI Area: Continuous Integration of RIOT components CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@miri64 @jia200x