-
Notifications
You must be signed in to change notification settings - Fork 20
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
Use arduino_ci for unit testing #9
base: master
Are you sure you want to change the base?
Conversation
a58c24e
to
d6b2985
Compare
d6b2985
to
9b6f213
Compare
|
||
# Generate and deploy documentation | ||
after_success: | ||
- source <(curl -SLs https://raw.githubusercontent.com/SMFSW/travis-ci-arduino/master/install.sh) |
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.
This line might not be necessary. It will save some build time if not, but I can't tell for sure since the behavior seems to be different for PR tests vs merges into a branch (and I can only trigger the PR tests as a contributor).
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.
Hello, I'm not sure what you did with .travis.yml file.
I just checked, after_succes doesn't call install.sh script, but check (if needed to be implemented later, then doxygen deploy script).
About saving some build time, I recently moved on to travis-ci.com and it caches parts of the VM installation (like boards for example), so it doesn't take so long now to execute everyhting.
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.
My apologies for lack of context here. An energetic discussion in arduino/Arduino#7567 this morning brought up #4, because it related to automated testing.
I noticed that in #4 you had said
The idea of having unit testing is seducing and as long as it remains free, I'm welcoming it, for this lib,
So I took some time this morning to attempt that seduction :) But since it's coming to you as noise from another thread, I apologize for the abrupt and unsolicited contribution.
To your question:
I'm not sure what you did with .travis.yml file
The 2 lines I added (bundle install && bundle exec arduino_ci_remote.rb
) invoke the unit test & compilation library that I wrote -- replacing the need for the Adafruit script to do this. As such, I removed build_platforms
rather than have the Adafruit script repeat it. However, my library doesn't do anything with documentation so I moved the installation of that library to the after_success
section in case it was needed for the 2 commands that follow it.
We can close this PR as-is; it has illustrated what I was hoping to show: functional CI that includes unit tests an compilation tests on an Arduino library (and they all pass 🎉).
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 am not so familiar with such scripts and I'm pretty happy with my fork of bash travis-ci-arduino.
At the moment, I have more time to spend developping my libs and improving them, rather than changing these scripts that actually works fine for the purpose intended (check that compilation works on various platforms, and most of all, deploying the documentation).
About Queue library itself, the examples and LibTst are good ways to verify for me that the lib works fine.... And even with all unit testing, the lib will still fail if anyone doesn't follow the doxygen warnings (disabling interrupts when popping an entry from a queue filled in some interrupt routine).
Thanks for the tips anyways, I will look about all this when I have some time, and thanks for your interest in Queue library!
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.
And thanks for the effort, I'll have a better look at is soon!
I'm happy to know that you managed to made this lib pass some unit testing without issues :)!
I saw about the discussion, scls19fr included me in it!
I'm sorry if my words seems (maybe) rude, this was not my intention at all, but I can agree with ladyada on her answers about unit testing, mostly when it comes to hardware related code.
Also, about unit testing, I tried it some at work too years ago, but nothing replaces a good debugger and proper means of manual testing when possible in my opinion. I mean, you generally can test some isolated functions with unit tests (which are abstracts), but for procedures, you can get serious headaches to manage testing them, if even it's possible ;)
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.
No offense taken on my end 😄it was good practice for me and I appreciate your comments. Please don't hesitate to close this if it's not right for the project at this time.
This PR provides a unit test that (for better or worse) attempts to line-for-line copy the LibTst.ino example. It asserts a variety of conditions related to (over)filling and (over)emptying a queue.
Fixes #4