-
Notifications
You must be signed in to change notification settings - Fork 16
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
perf: wait for the dbus-daemon to be ready using its output #152
Conversation
bdfe5c0
to
371e6f1
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #152 +/- ##
=======================================
Coverage 88.62% 88.62%
=======================================
Files 34 34
Lines 2532 2532
=======================================
Hits 2244 2244
Misses 221 221
Partials 67 67 ☔ View full report in Codecov by Sentry. |
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.
Good catch, I think we can be more idiomatic in our implementation - I've left some comments inline.
PS: Since these two functions were already pretty much the same, can we just maybe move them in a common place? If so, which one should I pick?
The testutils
package would be a good candidate for this IMO
IIRC when I tried that it didn't work because of an import loop.
|
b1affe8
to
1b8ff32
Compare
…g cycle When using testutils inside internal/brokers we were getting this: ❯ go build -C ~/Dev/authd/cmd/authd -tags withexamplebroker package github.com/ubuntu/authd/cmd/authd imports github.com/ubuntu/authd/cmd/authd/daemon imports github.com/ubuntu/authd/internal/services imports github.com/ubuntu/authd/internal/brokers imports github.com/ubuntu/authd/internal/testutils imports github.com/ubuntu/authd/internal/brokers: import cycle not allowed
dbus-daemon can print out its listen address when it's ready, by waiting so instead of just guessing how much to wait. Doing this through some goroutines although being this test code we could very likely just hang. Co-authored-by: Gabriel Nagy <[email protected]>
1b8ff32
to
fafc8bd
Compare
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.
This is really nice, thanks @3v1n0! I did a smoke test with the new implementation and confirm it works 🎉
dbus-daemon can print out its listen address when it's ready, by waiting so instead of just guessing how much to wait.
Doing this through some goroutines although being this test code we could very likely just hang.
PS: Since these two functions were already pretty much the same, can we just maybe move them in a common place? If so, which one should I pick?