Skip to content

tests/unittests/tests-core-clist: improve unit test#21402

Merged
maribu merged 2 commits intoRIOT-OS:masterfrom
maribu:tests/unittests/clist-sort
Apr 23, 2025
Merged

tests/unittests/tests-core-clist: improve unit test#21402
maribu merged 2 commits intoRIOT-OS:masterfrom
maribu:tests/unittests/clist-sort

Conversation

@maribu
Copy link
Copy Markdown
Member

@maribu maribu commented Apr 12, 2025

Contribution description

  • Iterate of different lengths of unsorted data to also cover corner case one-item list
  • Actually check that the list is sorted afterwards (and not just that the maximum got sorted in last)
  • Actually check that the list was stable-sorted (as the API doc promises a stable sort)

Testing procedure

  • The unit tests should pass
  • Mess with the sorting algorithm, check that the test stops passing

Issues/PRs references

None

@maribu maribu added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation 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 labels Apr 12, 2025
@maribu maribu requested a review from miri64 as a code owner April 12, 2025 09:00
@riot-ci
Copy link
Copy Markdown

riot-ci commented Apr 12, 2025

Murdock results

✔️ PASSED

60e3102 tests/unittests/core: reformat code

Success Failures Total Runtime
16 0 17 01m:47s

Artifacts

@maribu maribu force-pushed the tests/unittests/clist-sort branch from 144b2cc to 4d305ec Compare April 13, 2025 19:34
Copy link
Copy Markdown
Contributor

@crasbe crasbe left a comment

Choose a reason for hiding this comment

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

Disclaimer: I still don't know anything about clist :D

There are also two static errors about line length.

Copy link
Copy Markdown
Contributor

@mguetschow mguetschow left a comment

Choose a reason for hiding this comment

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

Nice one! Can we still get this merged today?

One of the commits (and the PR name) has a typo.

@crasbe crasbe added this to the Release 2025.04 milestone Apr 14, 2025
@mguetschow mguetschow removed this from the Release 2025.04 milestone Apr 14, 2025
@maribu maribu changed the title tests/unittests/texts-core-clist: improve unit test tests/unittests/tests-core-clist: improve unit test Apr 17, 2025
Copy link
Copy Markdown
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

Please squash

maribu and others added 2 commits April 23, 2025 20:39
- Iterate of different lengths of unsorted data to also cover corner case
  one-item list
- Actually check that the list is sorted afterwards (and not just that
  the maximum got sorted in last)
- Actually check that the list was stable-sorted (as the API doc
  promises a stable sort)
- Increase the length of the input data for better test coverage
- Check that no elements get lost while sorting

Co-authored-by: crasbe <[email protected]>
This should fix the column length warning of the static tests.
@maribu maribu force-pushed the tests/unittests/clist-sort branch from 0da3548 to 60e3102 Compare April 23, 2025 18:39
@maribu maribu enabled auto-merge April 23, 2025 18:41
@maribu maribu added this pull request to the merge queue Apr 23, 2025
Merged via the queue into RIOT-OS:master with commit d9938cc Apr 23, 2025
25 checks passed
@maribu maribu deleted the tests/unittests/clist-sort branch April 23, 2025 20:01
@maribu
Copy link
Copy Markdown
Member Author

maribu commented Apr 23, 2025

Thx!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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 Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants