Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: updating mocks for tests to test mender-connect JWT request handling #2108

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

chore: updating mocks for tests to test mender-connect JWT request handling #2108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 28 additions & 21 deletions tests/acceptance/mocks/mock_dbus_server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,29 +21,32 @@
class IoMenderAuthenticationIface:
"""
<node>
<interface name="io.mender.Authentication1">
<method name="GetJwtToken">
<arg type="s" name="token" direction="out"/>
<arg type="s" name="server_url" direction="out"/>
</method>
<method name="FetchJwtToken">
<arg type="b" name="success" direction="out"/>
</method>
<interface name="io.mender.Authentication1">
<method name="GetJwtToken">
<arg type="s" name="token" direction="out"/>
<arg type="s" name="server_url" direction="out"/>
</method>
<method name="FetchJwtToken">
<arg type="b" name="success" direction="out"/>
</method>
<signal name="JwtTokenStateChange">
<arg type="s" name="token"/>
<arg type="s" name="server_url"/>
</signal>
<method name="MockSetJwtToken">
<arg type="s" name="token" direction="in"/>
<arg type="s" name="server_url" direction="in"/>
</method>
<method name="MockSetJwtTokenAndEmitSignal">
<arg type="s" name="token" direction="in"/>
<arg type="s" name="server_url" direction="in"/>
</method>
</interface>
<arg type="s" name="token"/>
<arg type="s" name="server_url"/>
</signal>
<method name="MockSetJwtToken">
<arg type="s" name="token" direction="in"/>
<arg type="s" name="server_url" direction="in"/>
</method>
<method name="MockSetJwtTokenAndEmitSignal">
<arg type="s" name="token" direction="in"/>
<arg type="s" name="server_url" direction="in"/>
</method>
<method name="MockEmitSignal">
<arg type="b" name="success" direction="out"/>
</method>
</interface>
</node>
"""
"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not a big deal, though. But for the future:

Changing the indentation is unrelated to your changes. The commit would read much cleaner if we could have here only the method that we have added. If you want to modify the indentation it is best to do it on a separate commit.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh sorry, the formatting test wouldn't pass unless i changed the formatting. I thought it may have been something with the formatter being updated. Is there any way I can come around the failing test without changing the formatting of the other parts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh sorry, the formatting test wouldn't pass unless i changed the formatting. I thought it may have been something with the formatter being updated. Is there any way I can come around the failing test without changing the formatting of the other parts?

I am not aware of any recent change. We use a fixed version of the formatting tool.

But in any case you could have formatted the existing code first, commit that, then add your changes.

Again, this is not a big deal so don't spend more time on it.


def __init__(self):
self.token = ""
Expand All @@ -68,6 +71,10 @@ def MockSetJwtTokenAndEmitSignal(self, token, server_url):
self.JwtTokenStateChange(self.token, self.server_url)
return True

def MockEmitSignal(self):
self.JwtTokenStateChange(self.token, self.server_url)
return True


loop = GLib.MainLoop()
bus = SystemBus()
Expand Down
41 changes: 33 additions & 8 deletions tests/acceptance/test_mender-connect.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,15 @@ def dbus_set_token_and_url_and_emit_signal(connection, token, server_url):
)


def dbus_emit_signal(connection):
connection.run(
"dbus-send --print-reply --system "
"--dest=io.mender.AuthenticationManager "
"/io/mender/AuthenticationManager "
"io.mender.Authentication1.MockEmitSignal "
)


def wait_for_string_in_log(connection, since, timeout, search_string):
output = ""
while qemu_system_time(connection) < since + timeout:
Expand Down Expand Up @@ -193,14 +202,22 @@ def test_mender_connect_auth_changes(
"""Test that mender-connect can re-establish the connection on D-Bus signals"""

try:
dbus_set_token_and_url(connection, "token1", "http://localhost:5000")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this we should keep as it was. The test scenario here is that the server is up and running already when mender-connect starts.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happened here? I still believe that we should simulate that the server is up and running before mender-connect starts.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okey, since the tests fail due to timeout when we first set the token and url, and then let mender-connect start. But is successful when we do it after mender-connect has started. I will investigate the issue 👍


# start the mender-connect service
startup_time = qemu_system_time(connection)
lluiscampos marked this conversation as resolved.
Show resolved Hide resolved
connection.run(
"systemctl --job-mode=ignore-dependencies start mender-connect"
)

startup_time = qemu_system_time(connection)
# wait until the connection happens
wait_for_string_in_log(
connection, startup_time, 30, "Started Mender Connect service"
)

signal_time = qemu_system_time(connection)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we use signal_time below (line 224) when looking for "Connection established..." in the log?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh sorry my bad, fixed it now 👍

dbus_set_token_and_url_and_emit_signal(
connection, "token1", "http://localhost:5000"
)

# wait for first connect
_ = wait_for_string_in_log(
connection,
Expand Down Expand Up @@ -277,14 +294,22 @@ def test_mender_connect_reconnect(
"""Test that mender-connect can re-establish the connection on remote errors"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much of what I commented in the previous test case is applicable to this one, so I won't review it in detail just yet 👀


try:
dbus_set_token_and_url(connection, "badtoken", "http://localhost:12345")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, the serer is expected to be running before mender-connect starts


# start the mender-connect service
startup_time = qemu_system_time(connection)
connection.run(
"systemctl --job-mode=ignore-dependencies start mender-connect"
)

startup_time = qemu_system_time(connection)
# wait until the connection happens
wait_for_string_in_log(
connection, startup_time, 30, "Started Mender Connect service"
)

signal_time = qemu_system_time(connection)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed it now 👍

dbus_set_token_and_url_and_emit_signal(
connection, "badtoken", "http://localhost:12345"
)

# wait for error
if version_is_minimum(bitbake_variables, "mender-connect", "2.3.0"):
error_message = "connection manager failed to connect"
Expand All @@ -305,11 +330,11 @@ def test_mender_connect_reconnect(
)

dbus_set_token_and_url(connection, "", "")
kill_time = qemu_system_time(connection)
# kill the server and wait for error
with_mock_servers[1].kill()
kill_time = qemu_system_time(connection)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is probably unnecessary and I vote for reverting it. It is in effect the same thing, but capturing the time before the action (in this case before killing the server) makes it consistent across the test.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed it now :)

_ = wait_for_string_in_log(
connection, kill_time, 300, "error reconnecting:",
connection, kill_time, 300, "waiting for reconnect",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why won't in this case eventually come back to error reconnecting? What has changed in mender-connect for this case? 🤔

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on returns made in the the tests it returned waiting for reconnect given the test case, while the test was made to wait for error reconnecting which made it fail.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okey

)

# Signal the other server
Expand Down