th/clients-tests #130
Closed
th/clients-tests #130
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
|
LGTM |
- don't use "hash" for a local variable in python. The editor highlights it like a special python name. - don't use "settings" for Connection.settings. Name it Connection.con_hash. The name "settings" is over-used already. "con_hash" really is the nested dictionary that we expose/receive from D-Bus. If we would use libnm for it, it would be an NMSimpleConnection instance, but we don't.
lso, the name "connection" and "con" is overused. Use "con_inst" where we mean an instance of a "Connection" class, that is, the object exposed on D-Bus.
These names are unique and well-known.
Define all custom exception types together.
…sing libnm The real NetworkManager service has a clear understanding how a valid connection looks like. This is what nm_connection_verify() returns. Let also our stub-service verify connections the same way. Note that this is cumbersome, because the stub service uses python's dbus module, while libnm only accepts creating NMConnection instances from GVariant. Thus, we need to a cumbersome conversion first. It would be better if test-networkmanager-service.py would also expose normalized connections on D-Bus. But that requires the inverse converion from GVariant to python dbus.
Just to give it some variety. Also, note how the message from the server cannot be translated. Which is the case with real NetworkManager as well, and is a major usability issue.
lkundrak
force-pushed
the
th/clients-tests
branch
from
8f1fdf0
to
7c55c4d
Compare
|
Merged to master |
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.
Extend unit tests to verify connection using libnm.
This war rather cumbersome to do, because our
test-networkmanager-service.pyuses python's D-Bus library, while libnm uses GDBus. Hence, I first had to convert the dbus structure to a GVariant, which is surprisingly complicated.