tests: port pulseaudio test to session-tool #8581
Conversation
zyga
force-pushed
the
fix/pulseaudio-test
branch
2 times, most recently
from
9f778d8
to
86f14b0
Compare
| #shellcheck source=tests/lib/pkgdb.sh | ||
| . "$TESTSLIB"/pkgdb.sh | ||
| snap install --edge test-snapd-pulseaudio | ||
| truncate --size=0 defer.sh |
There was a problem hiding this comment.
s/defer/cleanup/, if we're looking for an analogy with Go, I would expect defer to run when prepare finishes, which is not the case.
There was a problem hiding this comment.
We have a few tests using this style. Would you mind if I did a follow-up that changes this in bulk?
There was a problem hiding this comment.
a followup sounds good to me
| systems: [ubuntu-1*-*64, ubuntu-2*-*64] | ||
| # Classic Ubuntu is sufficient to test the feature, | ||
| # except Ubuntu 14.04 where systemd is not supported. | ||
| systems: [ubuntu-16.04-*, ubuntu-18.04-*, ubuntu-19.10-*, ubuntu-20-*] |
There was a problem hiding this comment.
I suppose we can drop 19.10 at this point.
There was a problem hiding this comment.
Let's drop it when it's dropped from systems-*
| cat <<'EOF' > /home/test/pulse-test.pa | ||
| .fail | ||
| load-module module-null-sink sink_name=void | ||
| set-default-sink void |
There was a problem hiding this comment.
The test config set up a default null sink. Now we're back to the distro config which may use a real sound device when the test is run on real hardware by @sergiocazzolato
There was a problem hiding this comment.
I think that's fine if it works. This is better that it actually tests the distribution setting. Before we artificially injected the policy module so our test would pass if the actual Ubuntu config was buggy and would result in a security violation.
There was a problem hiding this comment.
I would rather us not use the distro config specifically because there may be bugs there. In general where it's not a lot of work (and in this case we have already done this work) replacing distro things that can become broken randomly at some point and thus are unstable sounds like a good thing for us, as we're not trying to test the distro we just want to test our things that interact with the distro. Making the distro less of a moving target is a good thing in my mind.
There was a problem hiding this comment.
I guess it depends on what we want to test, our own side or the integration of the distribution with our own side. Perhaps we just need to have a pair of tests (one with canned config, one with stock config).
There was a problem hiding this comment.
Btw, when the distribution drops the confinement patches I will say told you so :) This will catch a real regression :D
There was a problem hiding this comment.
ah you know I had honestly forgotten that the config we are specifying loads the snap policy module, I thought that was compiled into pulseaudio via patches and thus couldn't be turned off via config, but it appears that indeed you are right the mediation could be turned off via distro config... In this case you're right and I think we should be using distro config for this test.
There was a problem hiding this comment.
I would rather see us use our own config here where we already have that config written specifically because we are less exposed to bugs from the distro which leads to us spending more time wondering why this is broken. That being said, I don't feel strongly enough about it to block the PR, so a followup restoring that is fine, or I can just enjoy the feeling of saying "told you so" when the distro breaks us again evenutally.
| #shellcheck source=tests/lib/pkgdb.sh | ||
| . "$TESTSLIB"/pkgdb.sh | ||
| snap install --edge test-snapd-pulseaudio | ||
| truncate --size=0 defer.sh |
There was a problem hiding this comment.
a followup sounds good to me
| cat <<'EOF' > /home/test/pulse-test.pa | ||
| .fail | ||
| load-module module-null-sink sink_name=void | ||
| set-default-sink void |
There was a problem hiding this comment.
I would rather us not use the distro config specifically because there may be bugs there. In general where it's not a lot of work (and in this case we have already done this work) replacing distro things that can become broken randomly at some point and thus are unstable sounds like a good thing for us, as we're not trying to test the distro we just want to test our things that interact with the distro. Making the distro less of a moving target is a good thing in my mind.
This has failed so many times that restarting it probably costs more than the time required for fixing it. There are causalities, namely Ubuntu 14.04 which is no longer tested because it lacks systemd that really works. Deputy systemd handles only some services and is unavailable over DBus. Signed-off-by: Zygmunt Krynicki <me@zygoon.pl>
This has failed so many times that restarting it probably costs more
than the time required for fixing it.
Signed-off-by: Zygmunt Krynicki me@zygoon.pl