Skip to content

Conversation

@pabigot
Copy link
Contributor

@pabigot pabigot commented Oct 10, 2019

The default test now requires in excess of 16 K RAM to store the
interrupt table. Add the filter, and remove bbc_microbit (which
fails) from the whitelist.

Fixes build failure at https://app.shippable.com/github/zephyrproject-rtos/ci-test/runs/3154/3/tests

Copy link
Member

Choose a reason for hiding this comment

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

I fixed this recently so it'd fit in 16KB of RAM. I'd really like to keep the bbc:microbit in the list of tested boards. Can you reduce the number of connections instead? See 9be6fb0

@aescolar
Copy link
Member

@pabigot Do you know what caused the RAM use increase? (and why it was not detected in CI in that PR)

Reduce the RAM usage in hci_uart to fit in the BBC Microbit's RAM.

Signed-off-by: Peter Bigot <[email protected]>
@pabigot
Copy link
Contributor Author

pabigot commented Oct 10, 2019

Do you know what caused the RAM use increase? (and why it was not detected in CI in that PR)

No, and no. All I know is it failed in last night's sanitycheck --all run, and is the only test to have failed (though several matrix builds timed out, so more failures may be hidden). The nightly test has not passed for about a week and I keep getting bit by unrelated failures like this, so I'm trying to fix it.

Can you reduce the number of connections instead?

Done.

@jhedberg
Copy link
Member

It'd be good to understand what increased the footprint. This kind of gradual and uncontrolled upwards creep of it isn't particularly nice. Ideally we should have these memory-tight tests, like for the micro:bit, built on every PR so that footprint increases are caught immediately.

@pabigot
Copy link
Contributor Author

pabigot commented Oct 10, 2019

It started failing at 6700f2f from #17293. which was immediately followed by the commit that @carlescufi referenced to make it pass again. Will try again.

@pabigot
Copy link
Contributor Author

pabigot commented Oct 10, 2019

It started failing at 6700f2f from #17293. which was immediately followed by the commit that @carlescufi referenced to make it pass again. Will try again.

Nope, at 4b59690 it still fails. When #17293 was merged (with a passing CI) that test still failed, presumably because master had changed between the point the test was started and when the branch was merged and the decrease to 12 was already not enough.

The way Yocto manages merges is a lot more robust.

@carlescufi
Copy link
Member

It'd be good to understand what increased the footprint. This kind of gradual and uncontrolled upwards creep of it isn't particularly nice. Ideally we should have these memory-tight tests, like for the micro:bit, built on every PR so that footprint increases are caught immediately.

The problem is that we keep adding features that are enabled by default, which increases the footprint of default builds.

@aescolar
Copy link
Member

The problem is that we keep adding features that are enabled by default, which increases the footprint of default builds.

But still, I really though this test in this board was built in each CI run by default.
@carlescufi Would you be so kind as to add it so we catch their like in the future?

@pabigot
Copy link
Contributor Author

pabigot commented Oct 10, 2019

FWIW, these tests were run on bbc_microbit for that PR:

52523.1.log: 55/651 bbc_microbit              samples/boards/bbc_microbit/display/sample.board.bbc_microbit.display PASSED (build)
52523.1.log: 57/651 bbc_microbit              samples/boards/bbc_microbit/line_follower_robot/sample.board.bbc_microbit.line_follower_robot PASSED (build)
52523.1.log: 58/651 bbc_microbit              samples/bluetooth/ibeacon/sample.bluetooth.ibeacon PASSED (build)
52523.1.log: 59/651 bbc_microbit              samples/boards/bbc_microbit/sound/sample.board.bbc_microbit.sound PASSED (build)
52523.1.log: 60/651 bbc_microbit              samples/synchronization/sample.synchronization     PASSED (build)
52523.1.log: 61/651 bbc_microbit              samples/boards/bbc_microbit/pong/sample.board.bbc_microbit.pong PASSED (build)
52523.1.log: 64/651 bbc_microbit              tests/bluetooth/mesh/mesh.microbit                 PASSED (build)

so this test was not run in this case.

@carlescufi
Copy link
Member

The problem is that we keep adding features that are enabled by default, which increases the footprint of default builds.

But still, I really though this test in this board was built in each CI run by default.
@carlescufi Would you be so kind as to add it so we catch their like in the future?

I actually do not know how to do that. What defines the default set of tests that is run by CI by default?

@aescolar
Copy link
Member

What defines the default set of tests that is run by CI by default?

@nashif Is there some way to define that a given test on a given platform must be always run in a normal CI run?

@pabigot pabigot changed the title samples/bluetooth/hci_uart: add filter for size requirements samples/bluetooth/hci_uart: change bbc_microbit RAM demands to so test passes Oct 10, 2019
@pabigot
Copy link
Contributor Author

pabigot commented Oct 15, 2019

So this is amusing: This patch is not yet merged, but the nightly autotests are passing.

On master at 1ce95de we have:

tirzah[18]$ sanitycheck -T samples/bluetooth/hci_uart/
[...]
2 test configurations selected, 412 configurations discarded due to filters.
Adding tasks to the queue...
total complete:    2/   2  100%  skipped:    0, failed:    0
2 of 2 tests passed (100.00%), 0 failed, 0 skipped with 0 warnings in 8.06 seconds
In total 2 test cases were executed on 1 out of total 207 platforms (0.48%)

but

tirzah[16]$ sanitycheck -p bbc_microbit -T samples/bluetooth/hci_uart/
[...]
1 test configurations selected, 1 configurations discarded due to filters.
Adding tasks to the queue...
bbc_microbit              samples/bluetooth/hci_uart/sample.bluetooth.hci_uart.arm FAILED: Build failure
	see: sanity-out/bbc_microbit/samples/bluetooth/hci_uart/sample.bluetooth.hci_uart.arm/build.log
total complete:    1/   1  100%  skipped:    0, failed:    1

Looks like the new sanitycheck code ignores whitelist? @carlescufi @nashif

@pabigot
Copy link
Contributor Author

pabigot commented Oct 16, 2019

And shippable's failing again due to lack of this fix.

Can we merge this, or do we want to keep this failure present until somebody figures out how to make the test run in every sanitycheck?

@jhedberg jhedberg merged commit 336c90f into zephyrproject-rtos:master Oct 16, 2019
@pabigot pabigot deleted the nordic/20191010a branch October 19, 2019 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants