login1: Add NewWithConnection method #503
Conversation
This is a desired way of testing to avoid creating fragile test suites and be able to refactor code without touching tests. Signed-off-by: Mateusz Gozdek <[email protected]>
kolyshkin
left a comment
kolyshkin
left a comment
There was a problem hiding this comment.
It looks like there are API changes, meaning we'll need to target this to v23.
For v23, we can probably remove some deprecated stuff as well, and even rename some functions (but I don't have a plan yet).
Left a few nits, too.
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/coreos/go-systemd/v22/login1" |
There was a problem hiding this comment.
You can do
. "github.com/coreos/go-systemd/v22/login1"instead, to avoid modifications to the rest of the file.
There was a problem hiding this comment.
Thanks, although I am not sure I should do that. Conventionally, you write blackbox tests as you would be an external consumer of the code under test and most of the time code is imported into a separate namespace. I don't think skipping it buys a lot of extra readability in this case and optimizing the diff size doesn't seem like a convincing argument to derive from the convention to me.
Do you still want me to change that?
This method allows passing existing D-Bus connection, which allows to re-use connection between clients and to mock D-Bus connection for testing purposes. Signed-off-by: Mateusz Gozdek <[email protected]>
Closes coreos#388. Signed-off-by: Mateusz Gozdek <[email protected]>
invidian
force-pushed
the
invidian/new-with-connection
branch
from
5a0b55b to
d01e987
Compare
2 participants
This method allows passing existing D-Bus connection, which allows to
re-use connection between clients and to mock D-Bus connection for
testing purposes.
Extracted from #390
Includes commits from #395
Signed-off-by: Mateusz Gozdek [email protected]
This PR replaces #396, since the original fork has been archived and I am not able to update it. CC @kolyshkin