-
Notifications
You must be signed in to change notification settings - Fork 36
.github/workflow : Added compile tests #79
base: master
Are you sure you want to change the base?
Conversation
63dbf5d
to
eb1126e
Compare
ping @miri64 |
How can this be tested / what were the results? Feel free to squash btw. |
We had a small discussion at the summit. Maybe it would be better to use docker rather than a hard coded version of gcc. We could also try to run full build tests either but the When I have more time I will investigate. |
fadf0ba
to
a7cb6c8
Compare
@benpicco Here you go. results while doing a dry run in my system using act.
```
DRYRUN [Compilation tests for RIOT-OS submodule update/build-1] 🚀 Start image=catthehacker/ubuntu:act-latest
|
@MrKevinWeiss Do we really need to run the compilation tests on all targets from
|
You are right, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this works than I would be OK for now. I think there is too much info missing from the makefiles and such in order to use info-boards-supported
properly and try to test every one.
A simple native build should be at least better than nothing without adding too much extra work.
|
||
- name: Run compile tests | ||
run: | | ||
make BUILD_IN_DOCKER=1 -C ${{ matrix.applications }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried the following locally, this would all apps to be added without having to touch the CI list in order for testing.
make BUILD_IN_DOCKER=1 -C ${{ matrix.applications }} | |
export RIOTAPPS=$(find . -name '*Makefile*' -printf "%h\n" | sort -u | grep -v RIOT) | |
for appdir in $RIOTAPPS; do make BUILD_IN_DOCKER=1 -C $appdir; done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried the following locally, this would all apps to be added without having to touch the CI list in order for testing.
This would however disable parallel testing. I personally prefer to use a CI matrix if there is one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That means every test we would have to add into a list, also touching the github config, which also means that any non-maintainer would have to wait for it to be triggered.
Either way, AFAIK, we are merging this repo with the examples in the main RIOT repo. This would reduce all the duplicate CI work I suppose...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only 10% of changes in the past were file additions and not all of those were new applications (given that this repo only contains 3 applications at the moment)
$ git log --oneline --diff-filter=A | wc -l
14
$ git log --oneline | wc -l
140
I think we would manage ;-). But yes, given RIOT-OS/RIOT#18602 this might indeed be a mood point.
Added compile tests so that the compilation tests are triggered in the event that the submodule is updated.