login1: add NewWithConnection and RebootWithContext methods #390
Conversation
invidian
force-pushed
the
invidian/login1-reboot-tests-and-improvements
branch
from
0722452
to
b3aa1e3
Compare
invidian
force-pushed
the
invidian/login1-reboot-tests-and-improvements
branch
from
e187552
to
484c87e
Compare
1.12 is now EOL for almost 2 years: https://endoflife.date/go. As a reference, supported Ubuntu versions use either 1.10 or 1.13: https://packages.ubuntu.com/search?keywords=golang-go So I think this bump is reasonable. This is done to get access to stdlib methods like errors.Is and formatting statement %w, which is de facto standard for handling errors nowadays. Closes coreos#391 Signed-off-by: Mateusz Gozdek <[email protected]>
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]>
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]>
Existing Reboot() method does not allow using context not inspecting D-Bus call errors, which makes it difficult to debug and use. This commit adds new RebootWithContext() method which addresses those shortcomings. Closes coreos#387 Signed-off-by: Mateusz Gozdek <[email protected]>
invidian
force-pushed
the
invidian/login1-reboot-tests-and-improvements
branch
from
484c87e
to
1a186bc
Compare
|
Hey, thanks for quick feedback. Would you like me to create one PR per commit from this PR then? Generally, I'd prefer to not submit code without tests, no matter how trivial it is, and changes here as the simplest steps to get things testable. Also, I'm concerned that "everything else" will not get enough attention once |
|
Generally speaking yes, I prefer handling separate PRs as they can easily travel at different speeds and merge at different times. Regarding testing, it usually goes in the same PR, agreed. In this case though it is both a large reworking and pretty much just formalized mocking. It effectively does not cover much juicy content, other than package-global constants and other static strings. |
This PR does some small improvements to
login1package, namely changes tests to use blackbox approach to avoid building up the tech debt by adding new tests without doing so in the first place.Then adds
NewWithConnectionmethod together with tests as a new constructor to login1.Conn (#389), which allows re-using and mocking D-Bus connection so remaining method calls can be tested.Finally,
RebootWithContextmethod is added as an replacement and improved version ofReboot.Closes #387
Closes #391
Submitted code is based on the work I've done in flatcar/flatcar-linux-update-operator#112.