-
Notifications
You must be signed in to change notification settings - Fork 119
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.
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
Consider merging avr-100-pin branch into master #41
Comments
Sounds like a good idea to add Travis CI. I haven't found any new bug with my testing system for 5+ days now and have been using it a lot so I'm feeling more confident in it. I haven't really focused on using Travis with hardware packages other than the test I did on MCUdude_corefiles. I'm going to think about if there are any common tasks related to testing hardware packages that I should add to my script. I'm also planning to add a function to automate the Boards Manager JSON update process but that's a lower priority and would probably be specific to the gh-pages branch. Here's my preliminary plan for testing: Do you have any sketches other than those that you want Travis to compile? |
Nope, I think that's all. Ideally I want to get rid of the duplicate libraries and merge all MegaCore related dependencies into the official libraries. Haven't gone that path yet, but I think may be worth it in the long run. My only concern is that the official library almost seems to be abandoned. They all have lots of old unanswered issues and PRs. |
I have now set up a Travis CI configuration for the MegaCore avr-100-pin branch: Unfortunately the preliminary build revealed some problems:
So let me know how I should proceed. |
I just installed IDE v1.6.5r5 on my mac and renamed the arduino15 folder. It builds just fine, so I think it may be a linux related issue. Anyways I've clearly written in the readme file that IDE 1.6.12 or newer is recommended, so if someone complains about this issue they just have to update their IDE to a newer one. I suggest only testing on IDE 1.6.7 and newer. What's important here is that it's compatible with the newest IDEs. |
I think I'm close to having a working configuration. Unfortunately I had to break it into a bunch of jobs to keep the build from timing out. I wish I could figure out a good way to combine the reports. I think it would be a good idea for you to enable Travis CI for your repository before I submit the pull request so the PR will be tested on your repository to make sure it's set up correctly. Travis CI is free for public repositories.
After that, Travis will automatically build any commit or pull request to any branch of MegaCore as long as there is a .travis.yml file in the root folder. The current configuration in the .travis.yml file I wrote will cause you to get an email whenever a build succeeds or fails: If you like you can add a build status badge to your README.md: |
OK, 7 hours and 23000 compilations later the full build has completed! The build failed but the errors were not caused problems with the testing system or the IDE. Most were legitimate bugs with MegaCore:
The only problem with my Travis configuration that was revealed is:
So I need to change the configuration so that it doesn't compile the SoftwareSerial examples for ATmega64 and ATmega128. Full report is attached: |
This is great, thank you! I just created an account and followed you instructions of how to add MegaCore to travis. Do you have a |
It's here: So I'd recommend waiting to merge until I've found some sort of solution to the SoftwareSerial on ATmega128/64 problem. The legitimate bugs found by the test can be fixed separately but I'd like the test configuration to be correct before it's merged. You could merge to a test branch if you want to play around with it. Maybe you can find a good solution to these issues I'm having. I have no prior experience with Travis CI, Linux and shell scripts so there may an obvious solution I'm overlooking. |
Thanks, I'll wait until the issues are sorted out. Meanwhile I'll fix the bugs Travis found while building everything. This is really useful, as I'd never spot them myself. |
Yeah, with a project like this, with so many different variables to test, doing so manually is impossible, automation is the key! I knew there were a lot of combinations but I was amazed that it ended up being over 23000 and that was after limiting the IDE versions to 1.6.7 and newer. I'm confident that these tests give reasonably good coverage of the code. The main thing it's missing is a check of whether the bootloader files for all boards.txt configurations are present. The IDE does print a message but it's not an error so the script doesn't currently fail the build if that happens. I originally had more tests of the individual boards configuration options but ended up combining them with other tests where possible because they were causing the builds to time out. It's a difficult balance between full coverage and a reasonable build time. As it is now having to wait over 7 hours to see the results of a change is quite inconvenient. |
Ok, I'm back. Sorry for the delay. It's slow going with these long build times. Sometimes Travis CI is running slower than others so jobs that were just under the maximum time were going over and getting terminated. I finally have it broken into enough jobs where it doesn't time out but some of the jobs are still a bit close for comfort so I think I should split it up a little more to leave some room to grow. It's currently at 149 jobs and will be around 170 after I break it up more. The problem this presents is that it would be extremely tedious to try to manually copy that many reports together into one spreadsheet after every build. I wanted a solution that didn't require the user to sign up for another account at a different website. I know they already have a GitHub account so I had to figure out how to do it here. Travis CI has a system for deploying the build to GitHub pages but it force pushes every job to your repository, which erases the report from the last job. So that's completely worthless. The solution I settled on is to create a gist for the reports and have the script automatically add each report to the gist. It also adds a link to the reports in a comment on the commit, this is optional. GitHub displays the tab separated report as a searchable table. The entire gist can be easily downloaded so it's a much easier to put together a unified report, though you still have to concatenate the files. One major limitation is that, for security reasons, Travis CI doesn't allow the use of secure environment variables for pull requests from forks. This makes sense because otherwise someone could change the .travis.yml to do whatever they like with your GitHub personal access token and then run it by submitting a pull request. So the only way for reports from builds of commits to forks to be published is if the user generates a token and adds it as a secure environment variable in their Travis CI settings. Once the PR is merged to your repository the Travis CI build is run again and that build is able to publish a report as usual as it has access to your token. Here's the build: I merged your recent commits into my fork so now it's just errors compiling the SoftwareSerial examples that are causing the build to fail:
Unfortunately I've found the gist report solution has some problems that only showed after doing a large build:
These problems make me think that automatically committing to a branch of the repository (or a dedicated repository) would be a better solution. This would allow each build to be put in a separate folder (folders aren't allowed in gists), The reports would be shown as a list of files and then when you open any file it displays as a table. I'm going to add a function to my script for committing the report to a branch of a repository. I'll leave the one for the gist report, which might be a better option for smaller builds. So let me know what your preference is for the handling of the reports or if you have any ideas for another solution. I'm trying to make the script something that's generally useful for any Arduino project so any input from people who will be using it is helpful to me. The other question I have for you is how you would like to handle the use of my script by MegaCore. I wrote a script called arduino-ci-script.sh that attempts for provide functions for any common task necessary for testing Arduino projects:
The possible solutions:
So let me know your preference for the handling of the script, or if you have another idea. |
Just to make the question about your preferences for report publishing more clear: Report publishing options:
Report link comment options:
|
Thanks for all the hard work trying figuing this out. I really appreciate it, and I'm sure lots of Github/Arduino devs will benefit from you effort here 🔢 I need to be honest and ask; what do you think is the best option here? I understand that passing my token to a script I don't have control over is not a good idea. In my simple head the subtree option seems like a good idea. Then I have a physical copy of your script in the MegaCore repo, and can easily update it to the latest version. I don't believe there will be any security issues here either. I still need to manually add the .travis.yml at the root, right? Let's say we go for the subtree option. The procedure will go like this, right?
Are there anything I need to be aware of or take into account when I'm pushing new code to the MegaCore repo? Will it take 7 hours+ before I know if I broke the build or not? Is there anything "Pull requesters" need to be aware of? Anything I need to be aware of when merging a PR, except from the travis-ci script and the .travis.yml file? (Sorry if I've been a little slow with the messages lately. I've been quite busy, and it takes a little more time expressing myself in english rather than norwegian) |
I hope it will be useful to others. I started by wanting to do it for one of my repos, then decided I should do it for all my repos while I was at it. I realized that I was going to end up with a lot of duplicate code to maintain so I came up with the idea of the centralized script. You're having to go through the growing pains with me of finding all the issues with scaling up my original concept to this more demanding repository but once the kinks are worked out I think it will make these tasks easier for any other repo it's added to. I like the idea of the subtree. It makes sense that the script should be in this repository so it's completely self contained and not dependent on the existence of another repository. The clone option would make updating the script a little easier but you're already using sutrees in this repository so I don't think that's an issue.
Correct, just let me know what folder(s) you want it in and I'll set that up before I do the pull request.
Yes, it must be in the root of the repository
I think you already have done that following the instructions above but if you want the reports to be published you will also need to do the following:
I have step by step instructions for doing the above processes in the readme for my script but I'm adding a function to allow publishing reports to a repository to the script now so the instructions will soon be updated to reflect that change.
In some cases you may want to add new tests to MegaCore's .travis.yml file to cover the new code:
Unfortunately, yes. I got the build down to "only" 15000 compilations (from 23000) but the build time didn't go down. It may be that there is some overhead to each job so the reduction in compilations was offset by the increase in jobs or maybe Travis CI was running slow or they've throttled my account since I''m running so many builds. Possibilities for reducing the build time:
Depending on the changes made in the PR, additional jobs might need to be added to the .travis.yml file to ensure full testing coverage, as explained above. Ideally the PR would add any necessary tests but it's something that's easy to forget.
No worries! You're certainly better in English than I am in Norwegian! Sometimes I feel guilty that everyone else in computer land has to learn my native language. |
You've used Gist for this, right? I've not used Gist before, but if understood correctly it lives its own life, and does not affect any of the "ordinary" Github repos, right? It seems like the most "clean" option, because it live on it's own site. I assume that when it's first set up correctly it just "works" and doesn't need to be tweaked/changed if I'm doing changes to .travis.yml or the CI script? I'll take a closer look it it this evening |
I used Gist to publish reports for the last build of MegaCore: Gists are kind of like a piece of scratch paper. Say you needed to share some code with someone but it wasn't important enough to create a repository. Actually they're pretty powerful. They have a version history and can display markdown, csv, etc. They have comments. What I didn't realize before is they are actually repositories, you can clone them, commit, push, etc. but they don't support PRs or other collaboration, no issue tracker, etc. I've only used them for a couple things helping people on the forum but it's a useful tool sometimes. They even allow people to create anonymous gists without a GitHub account! I agree a gist seems the most clean. It seemed the best option for publishing the reports when I started the process. I don't really like the idea of having another branch of the repository that Travis CI is automatically committing to. I like the idea of having a separate repository for reports even less. Everything was looking very good for the gist approach until the MegaCore build of 150 jobs revealed some issues:
I think the gist feature of my script is still useful for some applications but MegaCore may not be a good match. The easiest option for a fork to get report publishing to work is if you use a branch of MegaCore for the reports. That way they only need to generate a token and configure it. I'm not sure that's an important consideration though because a lot of people who submit a PR probably won't bother with the full testing (though they really should), and creating a gist and configuring Travis CI with the gist ID will only take a couple minutes extra and is very easy following the instructions I provide in my readme. |
I understand that MegaCore might be to big for Gist. So, what do you think is the second best option? Maybe a dedicated branch Travis-build-outputs with a master branch with some info, and dedicated branches MegaCore, MightyCore, MiniCore and MajorCore? I don't like the idea of letting travis automatically commit to a branch either. How does the guys over at the esp8266 repo solve this? |
The only downside I see with a repository dedicated to reports is that if some one wants to publish reports from Travis builds on their fork they will have to create that repository but I don't think that will be a very common occurrence and it's very fast and easy to do. ESP8266 has a similar system in that they have a script that searches through the libraries folder and compiles every example sketch. They only compile for a single board with the hourly build of the Arduino IDE so it's a much less thorough test and it all fits in a single 17 minute job. They do have a report of the compile sizes of the sketches but from what I can tell it's only printed to the log. That's easy enough to work with when you have only one job per build. They are actually using arduino-builder to compile instead of the IDE. I had considered that because it might be a little faster but that means you can only test IDE versions 1.6.6 and newer and it doesn't reproduce the environment of the user as well. I have though about making the script using arduino-builder for IDE versions 1.6.6 and newer but I'd have to do some speed tests to see if it's worth all the extra complexity. I guess even a the difference of a second is huge when you're doing 15000 compilations per build! ESP8266 is using a service called codecov.io to show the coverage of their tests:
|
If contributors want to take advantage of Travis CI, I think wouldn't be a problem. I think I might go for this option, if you're OK with that. BTW i got the ATmega128 dev PCB in the mail the other day. Will sonder a few of them and ship one to you soon 🙂 |
Do you want the script to automatically comment a link to the report on each commit that is built with Travis CI?
Awesome! Thanks so much. I really appreciate it. |
I got some spare time today, so I'll dedicate some hours for this.
Like a link to the complete build output in the commit? I guess it's useful to have? If it isn't too much hassle though.
Maybe a folder Should I start off by creating the subtree of your CI script, or can a subtree be added from a PR? |
I've now added my personal Github token to the travis webpage using the guide over at your |
No trouble at all. The OK, so the script location will be travis-ci/arduino-ci-script.sh. I currently have it at arduino-ci-script/arduino-ci-script.sh in my fork:
Oops. I had originally set it up to use the
I'll submit a PR with everything ready to go. You've already done all the necessary configuration that it should correctly build your repository as soon as the PR is merged (the PR will build my fork using my token and report repository). I have a build running now of a .travis.yml configuration that I'm happy with. It hasn't revealed any fresh bugs in the script so I think I'm ready to submit the PR as soon as it finishes: |
I thought that too, so I decided to go for CI-reports myself. About the .travis.yml file. I'd like to get an email if the build fail, but I don't need to be notified if the build succeeded. Again, thanks for doing this for me, and the rest of the community. I'll play around with this tonight while watching Eurovision 🇳🇴 |
Comments would be made to the commits on the MegaCore repository. I believe this will cause anyone following MegaCore to get an email for every Travis CI build, which will happen every time you push commits (if you push multiple commits at once it only builds the most recent one).
OK, will do. |
Hmm. The author of the ArduinoJSON library uses multiple CI's. Travis or Codecov are commenting on every commit. Is is possible to disable these automatically generated messages? All the outputs can be found in the CI-reports repo anyway, right? |
Followers won't get emails for those, they're not really comments. I don't know what those are called.
Correct. I set it up to put the reports from each build in a separate folder so it's easier to find the right report. Without the comment the process for finding the reports from the build would be:
The build just finished! You can see the reports here: So I'll go ahead and submit a pull request now. |
Great! Do the PR to the avr-100-pin branch, and I'll merge that one into the master very soon 🙂 |
I've been working with the ATmega648/1280/2560 implementation for a while now, and other improvements like subtrees for the bootloader and core files have also been added. I'm sure the
avr-100-pin
brach still contains some bugs, but I'll have to merge it into themaster
branch in order to get it properly tested. I think it's a good idea to wait with a boards manager release until the dust have settled and (potentially) ugly bugs have been squashed.@per1234 if you want to create a PR to
avr-100-pin
and add Travis to MegaCore, I'd happily merge that. Now that this core is getting more and more complex in terms of dependencies, incorporating Travis seems to be a good idea. Any thoughts?The text was updated successfully, but these errors were encountered: