Skip to content

tests: remove invalid used memory assertion - #263

Open
massimomazzariol wants to merge 1 commit into
aparcar:mainfrom
massimomazzariol:fix-used-memory-assertion
Open

massimomazzariol wants to merge 1 commit into
aparcar:mainfrom
massimomazzariol:fix-used-memory-assertion

Conversation

@massimomazzariol

Copy link
Copy Markdown
Contributor

While looking at the base tests I noticed that test_free_memory() contains this assertion:

assert used_memory > 10000, "Used memory is more than 100MB"

free -m reports memory in MiB, so this currently requires the device to use more than 10 GB of memory, while the assertion message mentions 100 MB.

The test already stores the measured value in results_bag, and the commit that originally added it describes the purpose as storing used memory after startup.

This removes the inconsistent assertion and keeps the memory measurement in results_bag.

@aparcar

aparcar commented Sep 23, 2026

Copy link
Copy Markdown
Owner

Shouldn't we keep it an just fix the value?

@massimomazzariol

Copy link
Copy Markdown
Contributor Author

Shouldn't we keep it an just fix the value?

Sorry I just double-checked the original commit and I think my first assumption was wrong.
I initially thought the assertion could simply be removed because the value is still stored in results_bag so the memory measurement itself would still work but I noticed the assertion was added there from the beginning, so it was probably meant to be a real check too

The confusing part is that the condition uses > 10000 MiB while the message says "more than 100MB". Do you remember which one was intended? Was it supposed to fail when memory goes above 100 MB?

@aparcar

aparcar commented Sep 28, 2026

Copy link
Copy Markdown
Owner

100 (one hundred) MegaBytes should be exceeded in a stock firmware

Signed-off-by: Massimo Mazzariol <mazzariol.massimo@gmail.com>
@aparcar
aparcar force-pushed the fix-used-memory-assertion branch from 1870e52 to b96d66a Compare September 29, 2026 17:40

This branch has not been deployed

No deployments
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