Skip to content

riotctrl_shell.netif: add missing check_term decorator#14460

Merged
aabadie merged 2 commits intoRIOT-OS:masterfrom
miri64:riotctrl_shell.netif/fix/check_term-decorator
Jul 9, 2020
Merged

riotctrl_shell.netif: add missing check_term decorator#14460
aabadie merged 2 commits intoRIOT-OS:masterfrom
miri64:riotctrl_shell.netif/fix/check_term-decorator

Conversation

@miri64
Copy link
Member

@miri64 miri64 commented Jul 8, 2020

Contribution description

In #14441 I forgot to add the check_term() decorators to the methods that require it, which if not added one might run into a AttributeError since the term member of self.riotctrl is still None. This fixes that.

Testing procedure

tests-tools Github Action should still pass.

Issues/PRs references

Follow-up on #14441.

@miri64 miri64 added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Area: tools Area: Supplementary tools CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs labels Jul 8, 2020
@miri64 miri64 added this to the Release 2020.07 milestone Jul 8, 2020
@miri64 miri64 requested review from aabadie and bergzand July 8, 2020 10:08
@aabadie
Copy link
Contributor

aabadie commented Jul 8, 2020

Is there a way to check from a test that the decorator is correctly set ? Here there's a fix but no regression test is provided.

`check_term` calls `run_term` of the `ctrl` if necessary.
@miri64
Copy link
Member Author

miri64 commented Jul 8, 2020

Is there a way to check from a test that the decorator is correctly set ? Here there's a fix but no regression test is provided.

I've adapted the mock to act more like the real RIOTCtrl. Now if you revert HEAD~1 the tests should fail, while with HEAD~1 they succeed.

Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

Looks good and the CI checks are green. I trust your local tests about the non regression test (thanks for providing it btw).

ACK.

@aabadie aabadie merged commit 79f26bd into RIOT-OS:master Jul 9, 2020
@miri64 miri64 deleted the riotctrl_shell.netif/fix/check_term-decorator branch July 9, 2020 12:47
@miri64
Copy link
Member Author

miri64 commented Jul 9, 2020

Thanks!

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

Labels

Area: tools Area: Supplementary tools CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants