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

Modularize Tests #243

Merged
merged 27 commits into from Oct 16, 2020
Merged

Modularize Tests #243

merged 27 commits into from Oct 16, 2020

Conversation

erikarvstedt
Copy link
Collaborator

@erikarvstedt erikarvstedt commented Sep 24, 2020

This improves debugging and experimenting by making it easy to compose fine-grained
scenarios that have specific tests and features enabled.

Scenarios can now be defined like this:

# A node with two services. The corresponding tests are automatically enabled.
simple = {
  services.electrs.enable = true;
  services.clightning.enable = true;
};

# Full netns scenario, but with only two tests enabled
netnsTwoTests = {
   imports = [ scenarios.netns ];
   tests = mkForce {
     btcpayserver = true;
     netns-isolation = true;
   };
 };

Custom scenarios can conveniently be added by setting the env variable scenarioOverridesFile.

For undefined scenarios the tests are run with an adhoc scenario where services.<scenario> is enabled:

run-tests.sh -s clightning
run-tests.sh -s electrs

Also, the test output now includes the subtest name and duration and subtests use nested logging, which improves the html log.

Containers

Scenarios can now be run in containers (for now, without the python test suite) for super fast
debugging and experimenting.
Like in the examples, creating NixOS containers needs root permissions.

## Run single command in container, delete container afterwards
run-tests.sh -s bitcoind container --run c systemctl status bitcoind

## Start shell session in container, delete container afterwards
run-tests.sh -s bitcoind container
# Run command in container
c systemctl status bitcoind
# Run via SSH
cssh systemctl status bitcoind
# enter container
c
# enter via SSH
cssh

Review helper

Use the following to audit 4548576 (extra-container: pre-release -> 0.4)

tmp=$(mktemp -d -p /tmp)
mkdir -p $tmp/{old,new}
fetchRelease() { curl -SL https://github.com/erikarvstedt/extra-container/archive/$1.tar.gz | tar xz --strip-components=1 -C $2;}
fetchRelease 6cced2c26212cc1c8cc7cac3547660642eb87e71 $tmp/old
fetchRelease 0.4 $tmp/new
git diff --no-index $tmp/{old,new}
rm -rf $tmp

@erikarvstedt
Copy link
Collaborator Author

erikarvstedt commented Sep 24, 2020

@jonasnick, @nixbitcoin do you use a systemd-based distro other than NixOS for developing? I'll try adding extra-container support for these platforms then. (Or, preferably, I'd help you switch distros. 😸)

@jonasnick
Copy link
Member

Looks interesting! I'm running ArchLinux which is systemd based. So far qemu was sufficient for me but I wouldn't say no to testing with fast containers.

@erikarvstedt erikarvstedt force-pushed the modularize-tests branch 8 times, most recently from c843280 to ddead36 Compare September 28, 2020 11:10
Copy link
Member

@jonasnick jonasnick left a comment

Choose a reason for hiding this comment

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

This is excellent! Concept ACK

I don't see why the tests suddenly don't need to be executed in the corresponding namespace anymore, or was that always the case?

@nixbitcoin since you wrote a large part of the existing tests, would you have time to look at this as well?

test/tests.nix Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated
./run-tests.sh debug
print(succeed("systemctl status bitcoind"))

# Run a node in a container
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps mention that this is NixOS only?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll have a shot today at making extra-container work on all systemd-based distros.

Copy link
Member

Choose a reason for hiding this comment

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

This would also be interesting for me, since I'm running Debian and couldn't test the container yet. Planning to switch to NixOS this year.

test/tests.nix Show resolved Hide resolved
@erikarvstedt
Copy link
Collaborator Author

or was that always the case?

Yes.

@erikarvstedt
Copy link
Collaborator Author

@nixbitcoin it could be the case that the kernel in Debian 10.6 (latest release) is too old to support seccomp system call filters in container services, leading to failures when starting nix-bitcoin services.
I couldn't find a working, reasonably recent Debian Vagrant image, so could you check if this works?

run-tests.sh -s bitcoind container --run c systemctl status bitcoind

README.md Outdated Show resolved Hide resolved
@nixbitcoin
Copy link
Member

I couldn't find a working, reasonably recent Debian Vagrant image, so could you check if this works?

I'm getting /tmp/extra-container/bin/extra-container: line 21: errEcho: command not found

@erikarvstedt
Copy link
Collaborator Author

@nixbitcoin, thanks, I'll fix this in extra-container.
Probably the type -P machinectl check is failing on your system.
Fix this with sudo apt install systemd-container.

@nixbitcoin
Copy link
Member

./run-tests.sh -s bitcoind container --run c systemctl status bitcoind succeeded on Debian 10.6. I had to reinstall nix as a multi-user install.

@erikarvstedt
Copy link
Collaborator Author

erikarvstedt commented Oct 16, 2020

Awesome, thanks for testing. I've added a multi-user installation check to extra-container.

Use this to audit the extra-container update:

tmp=$(mktemp -d -p /tmp)
mkdir -p $tmp/{old,new}
fetchAndCheck() {
    local url=https://github.com/erikarvstedt/extra-container/archive/$1.tar.gz
    curl -sSL $url | tar xz --strip-components=1 -C $2;
    echo "sha256 for $1 is $(nix hash-path --base32 "$2")"
}
# old: 0lqdrd8pvsb5jf7v124n8r5dv9dikkqbzy8082wsdrgymn8s577b (sha256)
# new: 0gdy2dpqrdv7f4kyqz88j34x1p2fpav04kznv41hwqq88hmzap90 (sha256)
fetchAndCheck 0.5-pre-1 $tmp/old
fetchAndCheck 0.5-pre   $tmp/new
git diff --no-index $tmp/{old,new}
rm -rf $tmp

Meanwhile, I always keep updating the unsquashed branch.

$1 was not substituted due to single quotes.
Needed for container support.
This service is not enabled and its netns doesn't exist.
This fixes modules-only usage.

We can leave enabling tor and tor.client to secure-node.nix, on which
spark-wallet has a strict dependency.
This tests that the modules work without the secure-node template.

The test currently fails at runtime, but evaluating already helps
catching module-related errors.
Copy link
Member

@nixbitcoin nixbitcoin left a comment

Choose a reason for hiding this comment

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK 1cc432a1363eaafcb200b4ad3a4e7cad989fde26
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEV3o0Un8+KoXoD+Fk3RH5rVMIs7oFAl+JqWAACgkQ3RH5rVMI
s7psRw//SijO6jLSG0PvlTocU46FVGc2daJL1ZUf2aBcGAVfxe428t9fKjFqkccB
k3WK3qgF8T+Jlw2Jg61geihSu7ghJ2UJC2AG59Sq3jHopX+golwnEJTJV8ESWajO
o5ZVeA/zYtwPnN8yCjGpCSgHvhm2tlCWf0YUcMzAuwSp3qKLnlXlaF7HsMCTIxRU
vePnbzPJFSakc2SVW7tRaaycvMGpdKy4WEohv0kH0L5nvrbK3W5vmFi812UaFrxC
gMADX0wpoJm1CdZYvTWEIN3EC11x+SXkwn9wD8CxEX+MgDtUYXtc//JghQxDLUif
Dcr8J5JyFxK9Iyqv6I/wjQUpFmk4ELRH4JPUMaDhV6Pwu4Q+0tVqinwj8kWv2E2y
CdkehkETawWMtVL/lNtIpytcp3X+CDXx8kaYOO5Ul6uFUicgPsiOMSaeup4zcmWd
6aeZr6TyQzT3OhnIkbuxoNfFxzqdmaFNe5BUSNw/9DdSxUQLay4h0fceqiBdUdyc
2NOjYtg+7vHFpnx/0essWPWO8vqIMdb/5G0Tdzu5bVj4FpW8LPncMS6pAxoCjZO6
wHCoN0W9UH966fgIAMw7+WDu1q60IFy36/CAcyh9OQ9xzZwXE/YEthpwyZhLF6Eq
kVSK5K1M/tmaR2yVnSONPen2pAV6/72vLWqFnVOTB7xx5KXE/wM=
=ekbv
-----END PGP SIGNATURE-----

Copy link
Member

@jonasnick jonasnick left a comment

Choose a reason for hiding this comment

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

ACK 1cc432a

@jonasnick jonasnick merged commit 7906715 into fort-nix:master Oct 16, 2020
@erikarvstedt erikarvstedt deleted the modularize-tests branch December 15, 2020 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants