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.

Sign up for GitHub

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

Jump to bottom

tests/pkg_micro-ecc: blacklist wsn430 boards #9034

Merged
merged 1 commit into from
Oct 23, 2018
Merged

tests/pkg_micro-ecc: blacklist wsn430 boards #9034

merged 1 commit into from
Oct 23, 2018

Conversation

cladmi
Copy link
Contributor

@cladmi cladmi commented Apr 26, 2018

Contribution description

Test breaks on wsn430 boards with the node rebooting.

I suspect because micro-ecc does not have 16b support.
If that is the reason other 16bit boards may also be added there.

Issues/PRs references

Release testing.

@cladmi cladmi added Area: tests Area: tests and testing framework Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR labels Apr 26, 2018
@cladmi cladmi added this to the Release 2018.04 milestone Apr 26, 2018
@bergzand
Copy link
Member

bergzand commented May 1, 2018

@cladmi Do you want to merge this one now or wait for the package upstream?

@bergzand bergzand self-assigned this May 1, 2018
@cladmi
Copy link
Contributor Author

cladmi commented May 3, 2018 •
edited
Loading

My comment upstream was 1 year ago :) so no need to wait.

For this one, either merge it with only wsn430 or add the other 16bit/8bit boards.
I could at least add a comment on top to say they are removed because of this and that other boards can be added.

Tell me what you prefer.

@kaspar030
Copy link
Contributor

I propose rebasing this on #9081 and change to FEATURES_REQUIRED += arch_32bit, if the code actually requires that. If msp430 is broken, avr8 is probably too.

@cladmi cladmi added the State: waiting for other PR State: The PR requires another PR to be merged first label May 8, 2018
@cladmi
Copy link
Contributor Author

cladmi commented May 8, 2018

Yep would be better.

@miri64 miri64 added State: waiting for other PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 5, 2018
@cladmi cladmi force-pushed the pr/tests/pkg_micro-ecc/blacklist/wsn430 branch from a786ae5 to f886561 Compare October 21, 2018 16:43
@cladmi
Copy link
Contributor Author

cladmi commented Oct 21, 2018

I updated to include all 16bits platforms.

@jia200x this is a test that failed during release 2018.04, I think it could be merged for the current release.

@cladmi cladmi added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed State: waiting for other PR State: The PR requires another PR to be merged first labels Oct 21, 2018
@jia200x
Copy link
Member

jia200x commented Oct 22, 2018

@jia200x this is a test that failed during release 2018.04, I think it could be merged for the current release.

Sure, let's

@jia200x
Copy link
Member

jia200x commented Oct 22, 2018

@cladmi could you squash?

@jia200x
Copy link
Member

jia200x commented Oct 22, 2018

(we can update to #9081 in a follow up)

jia200x
jia200x approved these changes Oct 22, 2018
View reviewed changes
Copy link
Member

@jia200x jia200x left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

@jia200x
Copy link
Member

jia200x commented Oct 22, 2018

squash and green => GO

@cladmi
tests/pkg_micro-ecc: blacklist 16bits boards
69225e6 
Test breaks on wsn430 boards with the node rebooting.

This must come becauso `micro-ecc` does not have 16b support.
I also blacklisted the other 16bits boards.

 * kmackay/micro-ecc#96
@cladmi cladmi force-pushed the pr/tests/pkg_micro-ecc/blacklist/wsn430 branch from f886561 to 69225e6 Compare October 22, 2018 12:29
@cladmi
Copy link
Contributor Author

cladmi commented Oct 22, 2018

Squashed, waiting for murdock now.

@cladmi cladmi added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 22, 2018
@jia200x jia200x added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR labels Oct 22, 2018
@jia200x
Copy link
Member

jia200x commented Oct 23, 2018

Murdock reports success but GH didn't catch the status. I will retrigger

@jia200x jia200x added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 23, 2018
@miri64
Copy link
Member

miri64 commented Oct 23, 2018

The webhooks for that might not be up-and-running yet.

@miri64
Copy link
Member

miri64 commented Oct 23, 2018

Mhh no, the build is marked as failed in the overview as well...

@miri64
Copy link
Member

miri64 commented Oct 23, 2018

The build only failed the label check

Ok... I guess this is a bug in Murdock.... @kaspar030 @smlng @aabadie?

@smlng smlng added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 23, 2018
@smlng
Copy link
Member

smlng commented Oct 23, 2018

looks like there are still timing issues with GitHub for the web hook that informs about label changes.

@kaspar030
Copy link
Contributor

Hm, I have another hunch: We've renamed "Waiting for other PR" to "State: waiting for other PR", without first removing the old label from PR's if set. Current Murdock is still configured to use the old Labels, and they have never been unset for this PR.

Restarting Murdock would solve the issue. @smlng could you fix the "fail_labels" in murdock.toml and trigger the restart?

@cladmi
Copy link
Contributor Author

cladmi commented Oct 23, 2018

The static_test has been updated to use the new labels.

@cladmi
Copy link
Contributor Author

cladmi commented Oct 23, 2018

Oh I misread. So if I understand murdock is only updated of labels changes (so diff) by github and never queries the current value ?

@smlng smlng added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 23, 2018
@jia200x
Copy link
Member

jia200x commented Oct 23, 2018

btw Murdock is green ;)

@miri64
Copy link
Member

miri64 commented Oct 23, 2018

Hm, I have another hunch: We've renamed "Waiting for other PR" to "State: waiting for other PR", without first removing the old label from PR's if set. Current Murdock is still configured to use the old Labels, and they have never been unset for this PR.

Still feels like a bug to me, since the old labels were not used either.

@miri64 miri64 merged commit 3068fb8 into RIOT-OS:master Oct 23, 2018
@cladmi cladmi deleted the pr/tests/pkg_micro-ecc/blacklist/wsn430 branch October 24, 2018 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jia200x jia200x jia200x approved these changes

Assignees

@bergzand bergzand

Labels
Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer
Projects
None yet
Milestone
Release 2018.10
Development

Successfully merging this pull request may close these issues.
6 participants
@cladmi @bergzand @kaspar030 @jia200x @miri64 @smlng