Skip to content

unittests/tests-rpl_srh: various improvements#10300

Merged
jia200x merged 6 commits intoRIOT-OS:masterfrom
miri64:tests/enh/gnrc_rpl_srh_unittests
Nov 23, 2018
Merged

unittests/tests-rpl_srh: various improvements#10300
jia200x merged 6 commits intoRIOT-OS:masterfrom
miri64:tests/enh/gnrc_rpl_srh_unittests

Conversation

@miri64
Copy link
Copy Markdown
Member

@miri64 miri64 commented Oct 30, 2018

Contribution description

During my refactoring work of IPv6 reception I also improved upon the unittests for the RPL SRH handler, but I didn't find any other way to include them (also their usefulness, except for compile tests [but there also tests/gnrc_ipv6_ext can be used] are little), so here they are in a separate PR.

  • Some stack optimizations
  • Some refactoring
  • Adding some tests for error cases (!)
  • Rename to tests-gnrc_rpl_srh

Testing procedure

Run tests/unittests either as a whole, or just tests-gnrc_rpl_srh.

Issues/PRs references

Related to my refactoring work (see project, but not a dependency of it nor dependent on any PRs in it (but helpful for testing #10227 ;-)).

@miri64 miri64 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: network Area: Networking 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 Oct 30, 2018
@miri64 miri64 requested a review from cgundogan October 30, 2018 11:50
@miri64 miri64 mentioned this pull request Oct 30, 2018
Merged
@miri64 miri64 mentioned this pull request Nov 14, 2018
Merged
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 17, 2018

Ping @cgundogan?

@jia200x
Copy link
Copy Markdown
Member

jia200x commented Nov 22, 2018 •
edited
Loading

I'm not an expert in RPL :(
@cgundogan could you help with the Reviewed: 1-Fundamentals label (e.g tests make sense, etc) while I do the rest?

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 22, 2018 •
edited
Loading

You don't have to be an expert on RPL to read the API documentation of that function and see if the tests test all cases ;-).

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 22, 2018

Oh, this information might also be helpful to understand the unittests :-) https://tools.ietf.org/html/rfc6554#section-3

jia200x
jia200x reviewed Nov 23, 2018
View reviewed changes
@jia200x jia200x added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines labels Nov 23, 2018
@jia200x
Copy link
Copy Markdown
Member

jia200x commented Nov 23, 2018

vera++ doesn't report errors in this PR. Code style OK

@jia200x jia200x added the Reviewed: 3-testing The PR was tested according to the maintainer guidelines label Nov 23, 2018
@jia200x
Copy link
Copy Markdown
Member

jia200x commented Nov 23, 2018

tested on native and b-l072z-lrwan1. Works as expected

jia200x
jia200x approved these changes Nov 23, 2018
View reviewed changes
Copy link
Copy Markdown
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 jia200x merged commit 396f53c into RIOT-OS:master Nov 23, 2018
@miri64 miri64 deleted the tests/enh/gnrc_rpl_srh_unittests branch November 23, 2018 12:49
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 23, 2018

Thanks for the review! :-)

@aabadie aabadie added this to the Release 2019.01 milestone Dec 2, 2018
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

@cgundogan cgundogan Awaiting requested review from cgundogan

Assignees

No one assigned

Labels

Area: network Area: Networking 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 Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Milestone

Release 2019.01

Development

Successfully merging this pull request may close these issues.

3 participants

@miri64 @jia200x @aabadie