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

Use Travis CI for automated testing #300

Merged
merged 11 commits into from
Mar 9, 2019
Merged

Use Travis CI for automated testing #300

merged 11 commits into from
Mar 9, 2019

Conversation

per1234
Copy link
Contributor

@per1234 per1234 commented Mar 9, 2019

Every time a commit is pushed or a pull request submitted to the repository, a series of tests will be run on Travis CI. Reports for the compilation tests will be published to https://github.com/spencekonde/CI-reports/tree/ATTinyCore.

Note that, in the case of the commits that add the checks for trailing whitespace and true tabs, it may be useful to view the diff with changes to whitespace hidden in order to verify that no other changes were made. I have added links to those diff views in the list above.

This PR should start ATTinyCore's first Travis CI build. The expected result of this build is failure. This is due to some of the compilation tests failing from what I believe may be legitimate bugs. They also may be a case of "well, of course sketch x isn't going to compile for board y because of z". In that case, I need to adjust the Travis CI configuration so that it doesn't compile that sketch for that particular board configuration. If there are cases where it's not so obvious that the code shouldn't compile, it might be worth adding an #error directive to provide a more descriptive error message. So someone should probably review the failed jobs in the build and then let me know if I need to make any changes to this PR before this is merged. If you want to get a start on that before ATTinyCore's build finishes, you can refer to the latest build of my fork of ATTinyCore:
https://travis-ci.org/per1234/ATTinyCore/builds/503865414
That is a listing of all the jobs of the build. The ones with red X are the failed ones. In the job log, you'll see a bunch of lines that look something like this:
Clipboard02
The command is shown on one line, and then a summary of the result is shown on the next. The ones with the summary that says "arduino Exit Status: 1" had a compilation failure. In the screenshot above, line 720 indicates that the command at line 700 resulted in a failed compilation. The log is "folded" to make it easier to read. If you click the triangle on the left side of any command line, it will unfold that section of the log to show the output, where you will find the compilation error.

…led Travis CI build

This will cause the author of the PR to get a notification. The hope is that this will prompt them to resolve the issue that caused the build failure.
codespell uses a blacklist system. This results in much less false positives than the traditional whitelist-based spell checkers (as well as more false negatives).
Files which require the use of true tabs (keywords.txt and makefiles) are excluded from the check.
Files that don't end in a newline can result in confusing diffs.
@SpenceKonde SpenceKonde merged commit e5cb569 into SpenceKonde:master Mar 9, 2019
@SpenceKonde
Copy link
Owner

SpenceKonde commented Mar 9, 2019

Looking at the results - the first thing that jumps out is that a lot of the failures are relating to using slave/master specific I2C libraries on the 841 and 828, with inappropriate menu options selected - the I2C mode menu can be set to master, slave, or both. We should either test with "both" mode selected, or with master mode selected for every example except slave_receiver and slave_sender.

The reason for that menu option is that the compiler doesn't seem smart enough to optimize out the slave or master mode specific code when only the other mode is used, so if support for both is included, it wastes a lot of flash.

@SpenceKonde
Copy link
Owner

Yeah - it looks like that may be all of the failures (I found one other issue, but it was minor)

@SpenceKonde
Copy link
Owner

Now that I've merged this - how will I find and view the travis results?

@SpenceKonde
Copy link
Owner

Or should I have not merged this until you adjusted the travis specs to not try to build the wire master sketches with slave mode only selected, and not try to build the wire slave sketches with master mode only selected?

@per1234
Copy link
Contributor Author

per1234 commented Mar 10, 2019

Now that I've merged this - how will I find and view the travis results?

It seems like something is not working correctly with this repository and Travis CI.

Status not shown for PRs and commits

If you look at the commit history for my fork:
https://github.com/per1234/ATTinyCore/commits/travis
You can see that the newest commit (previous commits don't have a status indicator because they were all pushed at the same time and Travis CI only builds the newest commit when you do that) has a red X, which means it has a status of failure. While a build is ongoing there would be a yellow circle there. If the build passed, it would be a green check mark. But that's not happening in your commit history. The same should have happened in this PR. There should be a status indicator next to each item in the PR list (example) and another in each pull request thread.

But that's not happening in your repository for some reason. It's strange because the builds are running and showing up in your Travis CI account:
https://travis-ci.org/SpenceKonde/ATTinyCore/builds

No comment on failed PR from Travis Buddy

Another issue is that Travis Buddy didn't comment on this failed PR, as should have happened. I just did a test in my ATTinyCore fork, and it worked fine:
https://github.com/per1234/ATTinyCore/pull/1#issuecomment-471234876

Compilation reports not published

I also notice that the compilation reports were not published to your ci-reports repository. I currently have Travis CI configured to publish the reports to the ATTinyCore branch of that repository, and I have confirmed this is working for my fork:
https://github.com/per1234/CI-reports/tree/ATTinyCore

Does your CI-reports repository have a branch named ATTinyCore?

Things to investigate

I suspect the first two are related.

Please check whether https://github.com/SpenceKonde/ATTinyCore/settings/hooks has an entry for notify.travis-ci-org:
Clipboard01

Please check whether https://github.com/settings/applications has an entry for "Travis CI for Open Source":
Clipboard02

I'm certain that I didn't need to do anything manually to add that entry to my repository settings, and I'm fairly certain that I didn't need to do anything manually to add it to my account settings so this all should have been done automagically when you signed up for a Travis CI account and enabled it for ATTinyCore, but maybe something went wrong there.

@per1234
Copy link
Contributor Author

per1234 commented Mar 10, 2019

Or should I have not merged this until you adjusted the travis specs to not try to build the wire master sketches with slave mode only selected, and not try to build the wire slave sketches with master mode only selected?

I was thinking to get all the issues worked out before this was merged, but I can submit another PR for that change and any others that are needed.

@SpenceKonde
Copy link
Owner

Thanks

Okay, i created an ATTinyCore branch in my CI-Reports repo. I had forgotten that step.

hooks and applications both have entries for Travis CI

Interestingly, I got an email for the Travis failure.

Every failure I've looked at in the report from your test run looks like it was related to the Wire menu option issue described above.

@per1234
Copy link
Contributor Author

per1234 commented Mar 10, 2019

hooks and applications both have entries for Travis CI

Darn, I was hoping that might be it. I'll do some research and see if I can come up with any other possibilities.

Interestingly, I got an email for the Travis failure.

Well, at least something's working!

Every failure I've looked at in the report from your test run looks like it was related to the Wire menu option issue described above.

How about these:
https://travis-ci.org/SpenceKonde/ATTinyCore/jobs/504118327#L747

/home/travis/Arduino/hardware/ATTinyCore/avr/libraries/Servo/src/avr/Servo.cpp: In function 'void __vector_5()':
/home/travis/Arduino/hardware/ATTinyCore/avr/libraries/Servo/src/avr/Servo.cpp:1063:44: error: cannot convert 'volatile uint8_t* {aka volatile unsigned char*}' to 'volatile uint16_t* {aka volatile unsigned int*}' for argument '2' to 'void handle_interrupts(timer16_Sequence_t, volatile uint16_t*, volatile uint16_t*)'
   handle_interrupts(_timer1, &TCNT1, &OCR1A);
                                            ^

https://travis-ci.org/SpenceKonde/ATTinyCore/jobs/504118337#L862

/home/travis/Arduino/hardware/ATTinyCore/avr/libraries/SPI/SPI.cpp: In static member function 'static void tinySPI::begin()':
/home/travis/Arduino/hardware/ATTinyCore/avr/cores/tinymodern/pins_arduino.h:161:21: error: 'DDC1' was not declared in this scope
 #define USCK_DD_PIN DDC1
                     ^
/home/travis/Arduino/hardware/ATTinyCore/avr/libraries/SPI/SPI.cpp:263:25: note: in expansion of macro 'USCK_DD_PIN'
     USI_SCK_PORT |= _BV(USCK_DD_PIN);   //set the USCK pin as output
                         ^
/home/travis/Arduino/hardware/ATTinyCore/avr/cores/tinymodern/pins_arduino.h:162:19: error: 'DDB2' was not declared in this scope
 #define DO_DD_PIN DDB2
                   ^
/home/travis/Arduino/hardware/ATTinyCore/avr/libraries/SPI/SPI.cpp:264:25: note: in expansion of macro 'DO_DD_PIN'
     USI_DDR_PORT |= _BV(DO_DD_PIN);     //set the DO pin as output
                         ^
/home/travis/Arduino/hardware/ATTinyCore/avr/cores/tinymodern/pins_arduino.h:163:19: error: 'DDB1' was not declared in this scope
 #define DI_DD_PIN DDB1
                   ^
/home/travis/Arduino/hardware/ATTinyCore/avr/libraries/SPI/SPI.cpp:265:26: note: in expansion of macro 'DI_DD_PIN'
     USI_DDR_PORT &= ~_BV(DI_DD_PIN);    //set the DI pin as input
                          ^

Once I have a verdict on those errors, I'll submit a PR to update the Travis CI config and hopefully we'll get to a passing build.

@SpenceKonde
Copy link
Owner

SpenceKonde commented Mar 10, 2019 via email

@per1234
Copy link
Contributor Author

per1234 commented Mar 10, 2019

I see that the status indicator is now showing in the commit history!
https://github.com/SpenceKonde/ATTinyCore/commits/master
and that the reports are now being published:
https://github.com/spencekonde/CI-reports/tree/ATTinyCore
Maybe the status indicator issue with PRs will also have mysteriously resolved itself as well. After that, there is the question of Travis Buddy. I set it to only comment on PRs with failed builds (though there is an option to also comment for passing builds) so the only way to test it is to submit a PR that will cause a failed build (which hopefully my next PR will not. I could submit a dummy PR that will intentionally fail if you want to test it out, or you can just wait until the inevitable legitimate failing PR comes along.

I am in awe of your ability to dig through those reports and find the few examples of different types of failures.

It's possible that there are others that I missed, but any of those will be easy to see once the Travis CI build with my updated configuration finishes, as they won't be masked by all those spurious Wire errors.

@SpenceKonde
Copy link
Owner

Thanks!

I fixed the issue with pins_arduino.h for the tiny1634 - that one compiles for me on all the versions of the IDE that I tried, but there appear to be copies of iotn1634.h that only have DDRxn defines, not DDxn defines for the bits of the DDR registers.

Servo does not support the attiny43 - I've added a #error to that effect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants