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.

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

Add support for automatic tests (CI) #213

Open
majekw opened this issue Jun 21, 2017 · 10 comments
Open

Add support for automatic tests (CI) #213

majekw opened this issue Jun 21, 2017 · 10 comments
Labels
Maintainability Fixing this should improve the readablity/maintainabiity of Optiboot. No-binary-change This issue was resolved without the popular binaries changing at all.

Comments

@majekw
Copy link
Contributor

majekw commented Jun 21, 2017

It would be good to use some recent development practices in Optiboot:-)
Automatic tests should make maintaining of this project much easier.

What can be done:

  1. Check if everything compile (done in Add support for test compilation using Travis-CI #212 )
  2. Check if everything compile on different versions of avr-gcc tools
  3. Check if sizes are within limits (for example <512B for small chips)
  4. Check if sizes changed (useful for simple 'typo' and documentation fixes)
  5. Check for size grow/shrink against last commit or PR vs. master

There are free services which can be used for this purpose like https://travis-ci.org/ or https://circleci.com/
Not everything is achievable out of the box, but even implementing small subset of automatic tests should help in this project.

@per1234
Copy link
Contributor

per1234 commented Jun 21, 2017

  1. Check if sizes are within limits (for example <512B for small chips)

Won't the compilation fail if it doesn't fit in the boot section specified in the makefile? If so, this item is also done in #212.

  1. Check if sizes changed (useful for simple 'typo' and documentation fixes)
  2. Check for size grow/shrink against last commit or PR vs. master

What's the difference between 4 and 5?

Regarding 4/5, I'm visualizing this being done in each job by cloning https://github.com/Optiboot/optiboot.git to some subfolder and then redoing the same compilation there. Probably the most simple comparison would be to just run avr-size on both the generated .hex files.

I'd also like to suggest that the warning count be compared. New warnings should not be tolerated unless there is a good justification for it.

@majekw
Copy link
Contributor Author

majekw commented Jun 22, 2017

Check if sizes are within limits (for example <512B for small chips)

Won't the compilation fail if it doesn't fit in the boot section specified in the makefile? If so, this item is also done in #212.

You are right, it should fail at linking stage.

What's the difference between 4 and 5?

In 4. I think more about checking md5sum, to confirm that result is really the same. I think that's a lot of work to do on Optiboot involving some reorganization of code, move some sections between files etc. It's good to be sure that nothing is changed in real code. Or for example something could be changed in BIGBOOT targets, so others shoudn't change.

In 5. it's just comparison to check how real changes (or compiler) influence size of code.

About implementation of 4/5: I think about setting some external service which could aggregate data from many builds and targets and then put it back to commit/pull request as one nice comment or review. So, I don't want to compile multiple commits at one go to make comparison. I think that it's doable using Amazon Lambda and Dynamodb using free of charge limits, but it's another story :-)

I'd also like to suggest that the warning count be compared

Good idea!

@WestfW
Copy link
Member

WestfW commented Jun 26, 2017

Check if sizes changed (useful for simple 'typo' and documentation fixes)
Check for size grow/shrink against last commit or PR vs. master

I was hoping to reduce the incidence of .hex files in the repository, since they really clutter up the diffs, and since (at least theoretically) they could be included in the "releases" instead. Can the tools extract code from one place, and "comparison binaries" from elsewhere?

Are there any tutorials for travis-ci ?

@majekw
Copy link
Contributor Author

majekw commented Jun 26, 2017

Can the tools extract code from one place, and "comparison binaries" from elsewhere?

Yes, that's direction I want to go.
In my opinion, moving away hex files from repository would be a good move. Just keep it in releases.
It's possible to use Travis-CI also for building releases, but I don't think it would be worth spending time to code that.

As for documentation: it's at https://docs.travis-ci.com/
Setting up Travis is quite easy - you just need to login to Travis using Github credentials, allow some access for it using API key (you could revoke it anytime) and put appropriate .travis.yml file in repository. Everything else is done automatically.
Of course Travis is more oriented at some popular programming languages as Python, Ruby, JS etc, but with some effort you can test there anything.
If you connect your account with Travis before merging #212, merge will trigger tests/build.
You could see how it looks after testing for example here: https://travis-ci.org/majekw/optiboot/builds/245529638

@jrbenito
Copy link

jrbenito commented Aug 3, 2017

Can the tools extract code from one place, and "comparison binaries" from elsewhere?

Well, a script running in travis can download a release file and compare its .hex with the actual generated .hex.

Or, before compile travis do a checkout on the repository. One build step could be checkout release, recompile, checkout latest, compile then compare both.

A simples code can check, after compilation success, if the current build is made in Master branch and contains a git tag of version. If so, zip results and upload to proper release space. That´s what CI means ;)

@jrbenito
Copy link

jrbenito commented Aug 3, 2017

@majekw

Good work!

One question, in the early discussion you mentioned use PlatformIO. After that I notice you opted not use PlatformIO. Just satisfying my curiosity, is there a reason you followed other path?

@majekw
Copy link
Contributor Author

majekw commented Aug 5, 2017

@jrbenito, I thought about PlatformIO because it looked at first glance as some way to do tests. Then I learned more about Travis-CI and found out that it's easier and faster to get only Arduino, unpack avr-gcc tools and environment is ready. Less to download, less things to configure, everything is simpler, build is faster.

@WestfW WestfW added Maintainability Fixing this should improve the readablity/maintainabiity of Optiboot. No-binary-change This issue was resolved without the popular binaries changing at all. labels Dec 16, 2017
@majekw
Copy link
Contributor Author

majekw commented Aug 17, 2018

All thing are done from initial list and are in #246 (testing PR are not tested).
For now, sizes could be checked in build log on Travis' site, but I'm going to make Github App to integrate this to show table with compilation status and size comparison on commit checks. But even now it's useful to check how things change between commits (output is markdown compatible, so after copy&paste it would make a nice table. It even supports emoji, but this is currently disabled :) )

@WestfW , please install https://github.com/marketplace/travis-ci application on this repository to make it work. Choose Open Source plan to use it for free.

@majekw
Copy link
Contributor Author

majekw commented Aug 17, 2018

This is example output from size/compilation check (I screwed in this build some virtual boot targets) pasted into github, with emoji :-)
https://gist.github.com/majekw/bc71a6c34b4cc3d1ea75ff78b0b736a3

@majekw
Copy link
Contributor Author

majekw commented Oct 8, 2018

New update for Travis-ci: #257

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Maintainability Fixing this should improve the readablity/maintainabiity of Optiboot. No-binary-change This issue was resolved without the popular binaries changing at all.
Projects
None yet
Development

No branches or pull requests

4 participants