-
Notifications
You must be signed in to change notification settings - Fork 221
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.
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
Add other combinations to test Dbus #1376
Conversation
Build failed. ✔️ unit-test SUCCESS in 8m 44s |
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 pull request ready? I am asking because all the newly added tests are failing:
fedora-38 | not ok 266 dbus: session bus inside Ubuntu 16.04 in 1750ms
fedora-38 | # (from function `assert_line' in file test/system/libs/bats-assert/src/assert.bash, line 488,
fedora-38 | # in test file test/system/211-dbus.bats, line 108)
fedora-38 | # `assert_line --index 0 "$expected_response"' failed
fedora-38 | #
fedora-38 | # -- line differs --
fedora-38 | # index : 0
fedora-38 | # expected : ()
fedora-38 | # actual :
fedora-38 | # --
fedora-38 | #
fedora-38 | not ok 267 dbus: system bus inside Ubuntu 16.04 in 1993ms
fedora-38 | # (from function `assert_line' in file test/system/libs/bats-assert/src/assert.bash, line 488,
fedora-38 | # in test file test/system/211-dbus.bats, line 138)
fedora-38 | # `assert_line --index 0 "$expected_response"' failed
fedora-38 | #
fedora-38 | # -- line differs --
fedora-38 | # index : 0
fedora-38 | # expected : (<'253.10-1.fc38'>,)
fedora-38 | # actual :
fedora-38 | # --
fedora-38 | #
fedora-38 | not ok 268 dbus: session bus inside Ubuntu 18.04 in 1804ms
fedora-38 | # (from function `assert_line' in file test/system/libs/bats-assert/src/assert.bash, line 488,
fedora-38 | # in test file test/system/211-dbus.bats, line 167)
fedora-38 | # `assert_line --index 0 "$expected_response"' failed
fedora-38 | #
fedora-38 | # -- line differs --
fedora-38 | # index : 0
fedora-38 | # expected : ()
fedora-38 | # actual :
fedora-38 | # --
fedora-38 | #
fedora-38 | not ok 269 dbus: system bus inside Ubuntu 18.04 in 1761ms
fedora-38 | # (from function `assert_line' in file test/system/libs/bats-assert/src/assert.bash, line 488,
fedora-38 | # in test file test/system/211-dbus.bats, line 197)
fedora-38 | # `assert_line --index 0 "$expected_response"' failed
fedora-38 | #
fedora-38 | # -- line differs --
fedora-38 | # index : 0
fedora-38 | # expected : (<'253.10-1.fc38'>,)
fedora-38 | # actual :
fedora-38 | # --
fedora-38 | #
fedora-38 | # test suite: Tear down
fedora-38 | ERROR
fedora-38 | {
fedora-38 | "delta": "0:16:41.342908",
fedora-38 | "end": "2023-09-25 12:44:17.449838",
fedora-38 | "msg": "non-zero return code",
fedora-38 | "rc": 1,
fedora-38 | "start": "2023-09-25 12:27:36.106930"
fedora-38 | }
Not yet. I'm still working on it. I pushed it in order to check whether the failures were the same as the ones I was obtaining locally. |
3c03e47
to
998ce4c
Compare
Build failed. ✔️ unit-test SUCCESS in 8m 53s |
998ce4c
to
b8bc29d
Compare
Build failed. ✔️ unit-test SUCCESS in 8m 50s |
b8bc29d
to
20ac329
Compare
Build failed. ❌ unit-test RETRY_LIMIT in 8m 15s |
20ac329
to
e161d8b
Compare
Build failed. ❌ unit-test RETRY_LIMIT in 8m 07s |
e161d8b
to
9f035c5
Compare
Build failed. ✔️ unit-test SUCCESS in 8m 23s |
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.
Thanks for updating this pull request, @nievesmontero ! Did you delete test/system/220-environment-variables.bats
by mistake?
I did a rebase by mistake, and after that all the tests were failing, that's why I removed those new files from my branch. Anyway, it was just to see if the tests were failing the same without them, and apparently after removing them, only the fedora rawhide and 39 are failing. |
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.
Nice set of tests! We definitely want these. Just a few points to resolve and this should be good to go. Also, make sure to revert the removal of the 220-environment-variables.bats
file. This can't be merged otherwise.
test/system/211-dbus.bats
Outdated
expected_response="$(gdbus call \ | ||
--session \ | ||
--dest org.freedesktop.DBus \ | ||
--object-path /org/freedesktop/DBus \ | ||
--method org.freedesktop.DBus.Peer.Ping)" | ||
|
||
run --keep-empty-lines --separate-stderr "$TOOLBOX" run --distro arch sh -c 'gdbus call \ | ||
--session \ | ||
--dest org.freedesktop.DBus \ | ||
--object-path /org/freedesktop/DBus \ | ||
--method org.freedesktop.DBus.Peer.Ping' |
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.
These commands seem to reoccur regularly. Putting them into a global variable could prevent a potential typo?
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.
That makes a lot of sense. If @debarshiray agrees, I can create two global variables, one for the session bus and another for the system bus.
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.
Sure. Feel free to submit a pull request.
test/system/211-dbus.bats
Outdated
expected_response="$(gdbus call \ | ||
--system \ | ||
--dest org.freedesktop.systemd1 \ | ||
--object-path /org/freedesktop/systemd1 \ | ||
--method org.freedesktop.DBus.Properties.Get org.freedesktop.systemd1.Manager Version)" | ||
|
||
run --keep-empty-lines --separate-stderr "$TOOLBOX" run --distro arch sh -c 'gdbus call \ | ||
--system \ | ||
--dest org.freedesktop.systemd1 \ | ||
--object-path /org/freedesktop/systemd1 \ | ||
--method org.freedesktop.DBus.Properties.Get \ | ||
org.freedesktop.systemd1.Manager Version' |
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.
What is the motivation behind using org.freedesktop.DBus.Peer.Ping
to only test the session bus and org.freedesktop.DBus.Properties.Get org.freedesktop.systemd1.Manager Version
to only test the system bus? Can't we use the same method for both types of buses?
Having the same method for both could prevent potential problems if only of those methods were to break.
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.
From my quick testing on Fedora 38 Workstation, Get org.freedesktop.systemd1.Manager Version
works for both the user and system buses, but Ping
only works for the user bus because it leads to GDBus.Error:org.freedesktop.DBus.Error.AccessDenied
for the system bus.
We could definitely include Get org.freedesktop.systemd1.Manager Version
for the user bus tests.
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.
Feel free to submit a pull request with Get org.freedesktop.systemd1.Manager Version
for the user bus tests.
31c0dbf
to
4204f33
Compare
Build failed. ✔️ unit-test SUCCESS in 8m 34s |
4204f33
to
32b9165
Compare
Build failed. ✔️ unit-test SUCCESS in 8m 26s |
32b9165
to
13f57a2
Compare
Build succeeded. ✔️ unit-test SUCCESS in 8m 44s |
13f57a2
to
80e379c
Compare
Rebased on top of current |
Build succeeded. ✔️ unit-test SUCCESS in 9m 06s |
80e379c
to
f2881bb
Compare
Build succeeded. ✔️ unit-test SUCCESS in 10m 15s |
f2881bb
to
d403f6a
Compare
Build succeeded. ✔️ unit-test SUCCESS in 8m 55s |
d403f6a
to
f3aa40e
Compare
containers#1330 Signed-off-by: Nieves Montero <[email protected]>
f3aa40e
to
1318c9e
Compare
Build succeeded. ✔️ unit-test SUCCESS in 9m 25s |
Dbus is only run in the default container. We want to add more tests in order to run it on different OS.