riotctrl_shell: provide netif interactions and parsers #14441
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
miri64
added
Type: new feature
|
@jia200x @benpicco: it would be great if you could provide example outputs of
I think, if possible, one example per bullet point would be enough. |
aabadie
requested changes
Jul 6, 2020
There was a problem hiding this comment.
There are some nits reported by the static-tests (flake8 + typos). Also some questions regarding the names of classes. Why Netif ? This seems confusing since it's a wrapper around ifconfig, same with NetifSend, it could just be Txtsnd ?
I think you could also use pytest.parametrize to factorize some test functions. I'd like to see the new interactions and parser used in our real tests :)
Mostly because the module is called like this. My idea was to name the classes like the module. I could name the methods like the shell command if you prefer (which I mostly already have done for |
miri64
force-pushed
the
riotctrl_shell/feat/netif
branch
from
ceb33a7
to
1664f12
Compare
Okay, I decided to rename |
LoRa output was provided by @leandrolanzieri offline. |
(legacy) O-QPSKMR-O-QPSKMR-OFDMMR-FSK |
|
@benpicco Thanks! |
|
Added @benpicco's examples, moved examples to their own test suite, applied fixes according to new tests |
miri64
force-pushed
the
riotctrl_shell/feat/netif
branch
from
d489f17
to
fb2c101
Compare
|
Rebased to current master to include GitHub actions for tool testing: |
aabadie
reviewed
Jul 7, 2020
aabadie
approved these changes
Jul 7, 2020
Originally, the options and flags in the `netif` shell output were separated by two spaces. For later added flags this is not the case, making the parsing of those flags and options hard to impossible. This change adds those missing spaces + comments so it might not happen again in the future.
|
Squashed |
miri64
force-pushed
the
riotctrl_shell/feat/netif
branch
from
b947bab
to
f8d23b4
Compare
|
None of the failing tests check the |
miri64
added
CI: skip compile test
|
Thanks for the review @aabadie! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Contribution description
Probably our hardest shell command to parse and execute, now as a
riotctrlshell interaction. :-).This PR also contains some fixes to the output of
ifconfigto make it easier to parse. I can move them to a separate PR, if preferred, the options changed don't seem to have been checked in any tests however.Testing procedure
toxshould succeed forriotctrl_shell:Issues/PRs references
Follow-up on #11406