Group IMPLEMENTS sections in a clearer way

2 files changed, 85 insertions, 46 deletions

diff --git a/git.liw.fi.sh b/git.liw.fi.sh
index 8e3ecd0..56945e4 100644
--- a/git.liw.fi.sh
+++ b/git.liw.fi.sh
@@ -1,6 +1,13 @@
 # Shell library for running git.liw.fi ACL tests.
 
 
+# We create ssh keys in the test suite. The temporary directory,
+# $DATADIR, must be made inaccessible to others before that happens.
+# We do it here, so it gets done before any of the code from an
+# IMPLEMENTS actually runs.
+chmod 0700 "$DATADIR"
+
+
 # This is handy for giving an error message and aborting.
 
 die()
diff --git a/git.liw.fi.yarn b/git.liw.fi.yarn
index a469e6d..ba616b2 100644
--- a/git.liw.fi.yarn
+++ b/git.liw.fi.yarn
@@ -1,8 +1,7 @@
-Test suite for ACL on git.liw.fi
-================================
+% Test suite for ACL on git.liw.fi
 
 Introduction
-------------
+============
 
 This is a test suite for my Gitano ACL setup on git.liw.fi. It is run
 against either the real or a test instance of the setup. It requires
@@ -10,7 +9,7 @@ the person running it to have admin access on the Gitano instance,
 so the tests can create and remove users and repositories.
 
 Pre-requisites
---------------
+==============
 
 Yarn must be run with `--env` used to set the environment variables
 `GITANO` and `GITHOST`. `GITANO` must be the Unix user for the
@@ -29,7 +28,7 @@ remove them after the test suite. The users may get created and
 removed multiple times.
 
 User creation
--------------
+=============
 
 The admin must be able to create and remove a user.
 
@@ -52,54 +51,67 @@ A non-admin mustn't be able to create or remove users.
 	AND an ssh key for tstusr2
 	WHEN admin creates user tstusr
 	AND tstusr attempts to create user tstusr2
-	THEN gitano failed with error matching "You may not perform site administration"
+	THEN attempt failed with error matching "You may not perform site administration"
 	FINALLY remove user tstusr on server
 
-Implementations
----------------
 
-Verify that there are no test related users on the server. If there
-is, something's gone wrong in a previous run, and things should be
-cleaned up manually. Or another run of the test suite is going on, and
-we shouldn't interfere with that.
+Implementation sections
+=======================
+
+
+Check results of attempted operation
+------------------------------------
+
+Some scenario steps attempt to do something which may (or should)
+fail. This step verifies the result of such an attempt. It is
+intentionally named to be quite generic so we don't need to have
+multiple "foo failed with error..." steps.
+
+	IMPLEMENTS THEN attempt failed with error matching "(.*)"
+	grep "$MATCH_1" "$DATADIR/attempt.stderr"
+
+ssh key generation
+------------------
+
+Our test users need ssh keys. We generate these on the fly rather than
+storing them in git, so that if someone gets a copy of this test
+suite, they don't have keys that can, at least temporarily, access the
+gitano instance.
+
+The key is stored as `$DATADIR/$USERNAME.key` (for the secret key;
+public key adds `.pub` to the end of the pathname).
+
+We run `ssh-keygen` with `-N` to set an empty passphrase. This is OK
+for test keys that never leave the local system, because our shell
+library makes sure `$DATADIR` is inaccessible to anyone else.
 
-We split up the various assumptions so the implementation code
-doesn't make assumptions on user names, etc.
+	IMPLEMENTS GIVEN an ssh key for (\S+)
+	ssh-keygen -f "$DATADIR/$MATCH_1.key" -N ''
+
+Check for users on server
+-------------------------
+
+We check for users on the server at various stages. Those tests are
+collected here, since they're all quite similar. Since we do it in
+several IMPLEMENTS sections, we have a shell function in the shell
+library to contain the actual code.
+
+First of all, we need to verify that there are no test related users
+on the server. If there is, something's gone wrong in a previous run,
+and things should be cleaned up manually. Or another run of the test
+suite is going on, and we shouldn't interfere with that.
 
 	IMPLEMENTS ASSUMING no (\S+) user exists on server
 	if user_exists "$MATCH_1"
 	then
 		die "User $MATCH_1 exists on server, but shouldn't"
 	fi
-	
-Create an ssh key for a user. This is generated for a scenario,
-then discarded.
-
-	IMPLEMENTS GIVEN an ssh key for (\S+)
-	ssh-keygen -f "$DATADIR/$MATCH_1.key" -N ''
-	
-Create a user on the server. Only an admin should be able to do
-this, but anyone can try. Note that since we only care about
-usernames, we invent the real name and e-mail address.
-
-	IMPLEMENTS WHEN (\S+) creates user (\S+)
-	user_add "$MATCH_1" "$MATCH_2"
 
 Verify a user exists on the server.
 
 	IMPLEMENTS THEN user (\S+) exists
 	user_exists "$MATCH_1"
 
-Verify a user can access gitano (by invoking whoami).
-
-	IMPLEMENTS THEN user (\S+) can access gitano
-	run_gitano_as "$MATCH_1" whoami | grep "User name: $MATCH_1\$"
-
-Remove a user.
-
-	IMPLEMENTS WHEN (\S+) removes user (\S+)
-	user_del "$MATCH_1" "$MATCH_2"
-
 Verify a user doesn't exist on the server.
 
 	IMPLEMENTS THEN user (\S+) doesn't exist
@@ -108,20 +120,40 @@ Verify a user doesn't exist on the server.
 		die "User $MATCH_1 exists on server, but shouldn't"
 	fi
 
-Clean up user.
+Verify a user can actually access gitano (by invoking whoami). This is
+necessary to make sure that user creation added the user's ssh key;
+otherwise other test steps may fail for unrelated reasons and the test
+suite may interpret that wrongly. Further, we make sure the user's ssh
+key can access their account and not some other account.
 
-	IMPLEMENTS FINALLY remove user (\S+) on server
-	user_del admin "$MATCH_1"
+	IMPLEMENTS THEN user (\S+) can access gitano
+	run_gitano_as "$MATCH_1" whoami | grep "User name: $MATCH_1\$"
+
+User creation
+-------------
+
+An admin creates a user on the server. Since we need to have a
+separate step for when a non-admin attempts the same, we have a shell
+function to do the actual work. The shell function also sets the ssh
+key for the user.
+
+	IMPLEMENTS WHEN admin creates user (\S+)
+	user_add admin "$MATCH_1"
 
 Attempt to create a user; check later if it worked.
 
 	IMPLEMENTS WHEN (\S+) attempts to create user (\S+)
 	attempt user_add "$MATCH_1" "$MATCH_2"
 
-Check error message from latest gitano run that we assumed would fail.
+User removal
+------------
 
-	IMPLEMENTS THEN gitano failed with error matching "(.*)"
-	echo ==========================
-	cat "$DATADIR/attempt.stderr"
-	echo ==========================
-	grep "$MATCH_1" "$DATADIR/attempt.stderr"
+Admin removes a user.
+
+	IMPLEMENTS WHEN admin removes user (\S+)
+	user_del admin "$MATCH_1"
+
+Admin clean up user at end of scenario.
+
+	IMPLEMENTS FINALLY remove user (\S+) on server
+	user_del admin "$MATCH_1"