Merged
Conversation
cnsnyder
requested review from
a team and
wottop
and removed request for
a team
ptoscano
suggested changes
Nov 1, 2022
Contributor
ptoscano
left a comment
ptoscano
left a comment
There was a problem hiding this comment.
I like the idea... however it still fails :-D
Few things:
- wouldn't it be better to patch directly in
DBusServerStubProvider, so it applies automatically to all the D-Bus tests? - maybe I would create a single temporary directory within
/tmpwhere to store all the D-Bus server sockets for all the D-Bus tests; this way/tmpis not polluted with sockets on failure - I would only configure the directory in
DomainSocketServer, rather the whole address string; this makes it less fragile to change/patch
Contributor
Author
|
1/ I don't think it makes sense to move the patch to The PR worked for me before and works now, after the changes; I can't reproduce the CI failure. We'll see if it gets better. |
m-horky
force-pushed
the
mhorky/pytest-dbus-register-fix
branch
from
384660e to
d45009e
Compare
This could help us with debugging errors when a test crashes on CI, but not locally.
m-horky
force-pushed
the
mhorky/pytest-dbus-register-fix
branch
from
bb599af to
04ca71e
Compare
jirihnidek
reviewed
Nov 3, 2022
m-horky
force-pushed
the
mhorky/pytest-dbus-register-fix
branch
4 times, most recently
from
947a900 to
e5a8656
Compare
m-horky
force-pushed
the
mhorky/pytest-dbus-register-fix
branch
from
e5a8656 to
b3e6074
Compare
The markers were removed before, but the D-Bus tests tend to fail *sometimes*, so it is useful to keep this in. All D-Bus tests inherit from this class, so we only have to do it here in one place.
m-horky
force-pushed
the
mhorky/pytest-dbus-register-fix
branch
from
b3e6074 to
557ca9f
Compare
m-horky
force-pushed
the
mhorky/pytest-dbus-register-fix
branch
2 times, most recently
from
f1dfad0 to
b283fce
Compare
Contributor
|
Sounds good now, thanks! (and we hopefully know what's the cause now) One small thing: since we know what was the reason for the change, can you please amend the message of the commit "Fix failures of D-Bus' Register tests" to mention that? right now it still says "unknown reasons" |
Our test that checks if it is possible to connect to newly created socket started failing with 'Permission denied' when the code tried to use it. Pytest cannot write into '/run', moving it to '/tmp' solves the issue. Additionally, as a result of relatively recent D-Bus implementation changes, 'tmpdir' and 'dir' behave the same and create regular socket, not an abstract socket. Connecting with "\0" prefix (which signifies the abstract one) now fails and thus has been removed from the tests.
m-horky
force-pushed
the
mhorky/pytest-dbus-register-fix
branch
from
b283fce to
bae7ce3
Compare
This was referenced Dec 5, 2022
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Our test that checks if it is possible to connect to newly created socket started failing with 'Permission denied' when the code tried to use it. Pytest cannot write into '/run', moving it to '/tmp' solves the issue.