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.

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

Port gnome3 like tests to python and udisks2 #72860

Merged
merged 5 commits into from
Nov 7, 2019

Conversation

worldofpeace
Copy link
Contributor

@worldofpeace worldofpeace commented Nov 5, 2019

Motivation for this change

#72828

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

@ofborg ofborg bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Nov 5, 2019
Copy link
Member

@jtojnar jtojnar left a comment

Choose a reason for hiding this comment

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

It would be nice to have a function for gnome-desktop-testing-runner too.

nixos/tests/fwupd.nix Outdated Show resolved Hide resolved

# send a dbus message to activate the service
$machine->succeed("dbus-send --system --type=method_call --print-reply --dest=org.freedesktop.PackageKit /org/freedesktop/PackageKit org.freedesktop.DBus.Introspectable.Introspect");
machine.succeed("dbus-send --system --type=method_call --print-reply --dest=org.freedesktop.PackageKit /org/freedesktop/PackageKit org.freedesktop.DBus.Introspectable.Introspect")
Copy link
Member

Choose a reason for hiding this comment

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

Was the dbus API added or will it come later?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean in the testing driver? I don't think so, but I have some great ideas of all the things that should be added there. Currently they've tried to go for a 1:1 port from the perl one.

nixos/tests/packagekit.nix Outdated Show resolved Hide resolved
nixos/tests/udisks2.nix Outdated Show resolved Hide resolved
@worldofpeace
Copy link
Contributor Author

worldofpeace commented Nov 5, 2019

It would be nice to have a function for gnome-desktop-testing-runner too.

I can make it similar to #34987 (comment) but maybe makeGnomeInstalledTest and have them in one flat file like what I did for flashback

(description is Make NixOS test for gnome-desktop-testing-runner using software)

@worldofpeace
Copy link
Contributor Author

@jtojnar I've added a gnome-installed-tests.nix with the function and the tests.
That does make all the tests under nixosTests.gnome-installed-tests.*.

Would you prefer having everything in one file like I've done, or perhaps the colord = makeGnomeInstalledTests could be split out?

@jtojnar
Copy link
Member

jtojnar commented Nov 6, 2019

I think one file per test is still a good idea, some of them are slightly more complicated, recall also python-packages.nix. The really trivial ones could perhaps be created directly in all-tests.nix.

I am also not sure about the GnomeInstalledTests name. While it originated in GNOME project, it spread to various freedesktop projects as well.

@worldofpeace
Copy link
Contributor Author

I think one file per test is still a good idea, some of them are slightly more complicated, recall also python-packages.nix. The really trivial ones could perhaps be created directly in all-tests.nix.

I am also not sure about the GnomeInstalledTests name. While it originated in GNOME project, it spread to various freedesktop projects as well.

I just wanted the function name to express that it's just for projects that use gnome-desktop-testing.

I will change the structure though, python-packages.nix was kinda a mess and all-tests.nix is structured that way for a similar reason.

@FRidh
Copy link
Member

FRidh commented Nov 6, 2019

I am a bit confused by the moving of the tests. In the package set, I thought the plan was to get rid of the gnome3 namespace. Why not with tests then?

@jtojnar
Copy link
Member

jtojnar commented Nov 6, 2019

gnome-desktop-testing is just a single implementation of the test runner but the installed spec itself is not specific to GNOME. There is at least ibus-desktop-testing-runner. I plan to add that in #71442

@worldofpeace
Copy link
Contributor Author

From IRC:

@worldofpeace
jtojnar: what are the requested changes in #72860 (comment)? I just wanted a good overview of the installed tests specifically, and having them under an attr nixosTests.gnome-installed-tests.* just made that easy. No relation to the gnome3 attr deprecation

@jtojnar
worldofpeace I would just use nixosTests.installed-tests, and maybe pester upstream to drop gnome from its name when I have some time

@worldofpeace
jtojnar: did that 👍️

nixos/tests/udisks2.nix Outdated Show resolved Hide resolved

$machine->succeed("udisksctl info -b /dev/vda >&2");
$machine->fail("udisksctl info -b /dev/sda1");
with lzma.open(
Copy link
Member

Choose a reason for hiding this comment

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

Why space this around, "${stick}" is much shorter than the line below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

black fails on line length, as this becomes a nix store path so it demands I format it this way. We should probably make that more relaxed. Similarly I've had to format makeInstalledTest testscript injection specifically with concatenating multi-line strings so it doesn't error out on it not being pretty.

nixos/tests/installed-tests/gdk-pixbuf.nix Outdated Show resolved Hide resolved
nixos/tests/installed-tests/gnome-photos.nix Outdated Show resolved Hide resolved
nixos/tests/installed-tests/default.nix Outdated Show resolved Hide resolved
nixos/tests/installed-tests/default.nix Show resolved Hide resolved
};

extraTestScript = ''
# dogtail needs accessibility enabled
Copy link
Member

Choose a reason for hiding this comment

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

In the future, we should probably have a module for this. Depends on #54150

nixos/tests/installed-tests/ostree.nix Outdated Show resolved Hide resolved
nixos/tests/installed-tests/default.nix Outdated Show resolved Hide resolved
, testConfig ? {} # Config to inject into machine
, extraTestScript ? "" # Test script snippet to inject before gnome-desktop-testing-runnner begins
, withX11 ? false # Does test need X11?
, testRunnerFlags ? "" # Extra flags to pass to gnome-desktop-testing-runner
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should make this an array in line with structuredAttrs proposal. I can imagine python test driver to accept list of args like Python’s subprocess module does.

nixos/tests/installed-tests/default.nix Outdated Show resolved Hide resolved
@worldofpeace
Copy link
Contributor Author

@GrahamcOfBorg test udisks2 fontconfig-default-fonts packagekit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants