The test following test is flaky:
TEST_ASSERT(msg->send_time > 0U);
It assumes that 0 as time is indicating the time is invalid. But on
`native`, the time will be zero for the first millisecond the test
is launched. Before the interactive sync caused seemingly a reliable
delay of at least one milli second, but this no longer is the case.
This works around the bug by just waiting at the start of the test
until the time is no longer zero, possibly spinning for one millisecond.
The aim is to allow faster test cycles on native for unit test style
apps (that don't need interaction) by bypassing the python test
framework using e.g.:
make RIOT_TERMINAL=native all term
This would work already before, but now is more convenient as no
manual press of the `s` key is needed to start the test.
For non-native boards we need the sync, as otherwise the board may
finish booting before the python test automation framework can capture
output. For `native` and `native64`, we actually control when the RIOT
app is started and do not need to sync.
On a typical machine this can reduce the test cycle by more than 4
seconds.
With this change:
$ time sh -c 'make BOARD=native -C tests/unittests tests-nanocoap -j && make BOARD=native RIOT_TERMINAL=native -C tests/unittests term'
[...]
main(): This is RIOT! (Version: 2024.10-devel-394-gd65dec-tests/no-sync-control)
...................................
OK (35 tests)
[...]
make: Leaving directory '/home/marian.buschsieweke@ml-pa.loc/Repos/software/RIOT/master/tests/unittests'
sh -c 0.30s user 0.24s system 113% cpu 0.476 total
Before t his change:
$ time sh -c 'make BOARD=native -C tests/unittests tests-nanocoap -j && make BOARD=native -C tests/unittests test'
[...]
main(): This is RIOT! (Version: 2024.10-devel-394-gd65dec-tests/no-sync-control)
Help: Press s to start test, r to print it is ready
READY
s
START
...................................
OK (35 tests)
[...]
make: Leaving directory '/home/marian.buschsieweke@ml-pa.loc/Repos/software/RIOT/master/tests/unittests'
sh -c 0.50s user 0.37s system 17% cpu 4.863 total
This changes the API of xfa from
XFA(array_name, prio) type element_name = INITIALIZER;
to
XFA(type, array_name, prio) element_name = INITIALIZER;
this allows forcing natural alignment of the type, fixing failing tests
on `native64`.
This disables adding/removing line breaks by `clang-format`, as this
could very well introduce regressions in code formatting in terms of
our coding convention: If a long array was matching the soft limit of 80
chars per line, `clang-format` would reflow it to match the hard limit
of 100 chars.
One can selectively disable `clang-format` with magic comments. But at
this point, I'd say we should rather disable this feature until we
can configure it in a way to match our coding convention better (without
having to add magic comments).
Before the test used the `help` command to verify that access to the
shell is present. While this does have the benefit of not requiring a
custom command, it does have some robustness issues:
- When new default commands get added, this test will fail
- When the help message of a command gets reworded, the test will fail
- When the order of commands in the test will change, the test will fail
- Note that with the XFA based `SHELL_COMMAND()` macro, we do not
guarantee any particular order. Specifically, the order can change
with implementation details.
This increases the test coverage for XFA:
- Multiple entries are added from a single file
- Priorities are assigned to enforce a given order
- The size of the `struct` to add is changed to no longer be a power of
2
- Allow specifying an alternative board filter for the bootloader TTY,
as this will present different vendor, model, and serial than RIOT's
USB implementation.
- Allow specifying the serial. This is useful when multiple CDC ACM
devices are present.
Before, handlers writing blockwise transfer assumed that the response
header length will match the request header length. This is true for
UDP, but not for TCP: The CoAP over TCP header contains a Len field,
that gets extended for larger messages. Since the reply often is indeed
larger than the request, this is indeed often the case for CoAP over
TCP.
Note: Right now, no CoAP over TCP implementation is upstream. However,
getting rid of incorrect assumptions now will make life easier
later on.
This increases test coverage for coap_build_reply_header() for the case
that extended tokens are used. The test validates the correctness of
the return value and the correctness of the assembled reply.
In case no payload is added, `coap_build_reply_header()` would return
`sizeof(coap_hdr_t) + token_length` regardless of the actual header
length returned by `coap_build_hdr()`. These can be different if
RFC 8974 extended tokens are enabled (module `nanocoap_token_ext`
used): If an extended token length field is used, its size is not
considered.
Co-authored-by: benpicco <benpicco@googlemail.com>
This makes an implicit agreement that code should include the headers
is uses, no more and no less, explicit.
It also recommends using tooling (such as clangd) and adding IWUY
pragma comments to aid those tools in common cases they otherwise would
provide false positives.
Co-authored-by: chrysn <chrysn@fsfe.org>
gcoap contains a hack where a `coap_pkt_t` is pulled out of thin air,
parts of the members are left uninitialized and a function is called on
that mostly uninitialized data while crossing fingers hard that the
result will be correct. (With the current implementation of the used
function this hack does actually work.)
Estimated level of insanity: 😱😱😱😱😱
This adds to insane functions to get the length of a token and the
length of a header of a CoAP packet while crossing fingers hard that
the packet is valid and that the functions do not overread.
Estimated level of insanity: 😱😱😱
The newly introduced insane functions are used to replace the old
insane hack, resulting in an estimated reduction of insanity of 😱😱.
Side note: This actually does fix a bug, as the old code did not take
into account the length of the extended TKL field in case of
RFC 8974 being used. But that is a bug in the abused API,
and not in the caller abusing the API.
Previously the corner case when RFC 8974 extended TKL fields are used
the result of coap_get_totel_hdr_len() was not tested, resulting in a
bug slipping through the test. This increase the test coverage.
