Port tests to Python #72810
Port tests to Python #72810
Conversation
ofborg
bot
added
6.topic: nixos
nixos/lib/test-driver/test-driver.py
Outdated
| @@ -642,6 +642,14 @@ def wait_for_x(self): | |||
| if status == 0: | |||
| return | |||
|
|
|||
| def copy_file_from_host(self, source, target): | |||
| """Copies the file located at source path to the target path. | |||
| Note: "source" is equivalent to "from", "target" is equivalent to "to" in the Perl script | |||
There was a problem hiding this comment.
Comments that reference the perl code lose their relevance as soon as we ported them all and deleted the perl code.
There was a problem hiding this comment.
I'm not sure if we should encourage copying files from the host in general (see my later comment). This might not work with binary files, too.
We should inline the self.succeed call in places where we really have to use it.
There was a problem hiding this comment.
Moreover this function is only used in the openssh test, so I'll remove the change completely.
nixos/tests/openssh.nix
Outdated
| server_lazy.copy_file_from_host("key.pub", "/root/.ssh/authorized_keys") | ||
|
|
||
| client.succeed("mkdir -m 700 /root/.ssh") | ||
| client.copy_file_from_host("key", "/root/.ssh/id_ed25519") |
There was a problem hiding this comment.
client.succeed should return the command output. Could you try generating the key on the client, read the pubkey in the test script, and copy it to the server that way?
There was a problem hiding this comment.
Yes, I've pushed the changes.
nixos/tests/tor.nix
Outdated
| $client->succeed("echo GETINFO version | nc 127.0.0.1 9051") =~ /514 Authentication required./ or die; | ||
| client.wait_for_unit("tor.service") | ||
| client.wait_for_open_port(9051) | ||
| if not "514 Authentication required." in client.succeed( |
There was a problem hiding this comment.
Can't we use assert here?
assert "514 Authentication required." in client.succeed("echo GETINFO version | nc 127.0.0.1 9051")and it should raise an AssertionError?
There was a problem hiding this comment.
Yeah, foo or die idiom should be rewritten to assert foo
marijanp
force-pushed
the
port-tests-python
branch
from
3b15b8b to
7d98a8b
Compare
|
LGTM. Do you want to port more tests in here, or should we merge this in? |
|
@flokli many many more are following... i suggest merging early so the PRs don't get too long. Also, this avoids rebasing conflicts and other hassle. |
Port tests to Python (cherry picked from commit 54c0ac5)
Janik-Haag
added
the
12. first-time contribution
Motivation for this change
Now that PR #71684 (nixos: add python testing support) by @tfc got merged, I would like to port some/all of the tests to Python.
Things done
Ported the copy_file_from_host function to Python.
Ported several tests to Python, which also conform the Black code style.
Tests have been run and all of them succeed.
sandboxinnix.confon non-NixOS linux)nix-shell -p nix-review --run "nix-review wip"./result/bin/)nix path-info -Sbefore and after)Notify maintainers
cc @