This repository has been archived by the owner on Sep 7, 2020. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 32
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
arnout
force-pushed
the
feature/tests_sniffer
branch
from
April 2, 2020 10:25
4e81fb6
to
8df84f7
Compare
arnout
force-pushed
the
feature/tests_sniffer
branch
2 times, most recently
from
April 2, 2020 10:33
81fc982
to
b5e7127
Compare
Galvien
approved these changes
Apr 2, 2020
Comment on lines
10
to
15
'''Static class that encodes the global options.''' | ||
verbose = False | ||
tcpdump = False | ||
tcpdump_dir = '' | ||
stop_on_failure = False | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this common practice? while it's convenient to keep these options on a static class I would assume they would be stored in either a tests.config
, a json file file or something of that sort.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A static class is a somewhat pythonesque way of organising global variables, yes. It has the advantage that you don't need the global
keyword.
tomereli
approved these changes
Apr 5, 2020
The Sniffer class is an abstraction of tcpdump. It's a generic implementation where the bridge interface is passed in as an argument. To make things easier, the directory where sniffer results are saved is added as a member of opts. Add the new Python module to the PEP8 checks. Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <[email protected]>
Create a new environment module with method launch_environment_docker. For now, it just initialises the sniffer and runs the test_gw_repeater script to start the containers. wired_sniffer is defined as a global variable in the environment module, so it can be accessed easily as env.wired_sniffer. unique_id and skip_init are now passed as arguments to the init function. Thus, the self.opts member is no longer needed. Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <[email protected]>
These are just oneliner functions, and called only once, so remove them. Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <[email protected]>
This fixes most of the flake8 issues in the script. Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <[email protected]>
Some of the long lines can be avoided (and become more readable) by adding additional variables. Some lines are still too long so add noqa E501 to them. Also comments can be rewrapped. Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <[email protected]>
Some of the too long lines don't look better after splitting, so add noqa E501 exceptions for them. Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <[email protected]>
Some of the too long lines can be overcome by splitting the line before .format. autopep8 doesn't do this automatically because this way of splitting is a bit controversial. While we're at it, avoid the use of **locals() since it's also a bit controversial. Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <[email protected]>
Check the MID of the topology query message. Discovered by flake8 (mid was unused). Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <[email protected]>
Now test_flows.py is PEP8 compliant, we can run the check on all files under tests. Use git-ls-files to do the pattern matching. Use -z (zero-separate) for good measure, even though spaces are not allowed in python module names so it shouldn't be necessary. Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <[email protected]>
arnout
force-pushed
the
feature/tests_sniffer
branch
from
April 6, 2020 10:58
b5e7127
to
5aa9d6e
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This is yet another spinoff of #1031.
It is here to allow @Galvien to rebase #1020 on top of it and put the new tshark features in the sniffer class.
Also, at @Galvien's request, make test_flows.py PEP8-compliant now.