some small convenience changes for tests

---
type: change
message: |-
  some small convenience changes for tests

  A few small fixes to the dehub package tests, to remove some boilerplate and
  make the harness more flexible.
change_hash: AGSCUBN1O0Nbj77jWn17FE8rw6BfOZIm99d5gT+32pcs
credentials:
- type: pgp_signature
  pub_key_id: 95C46FA6A41148AC
  body: iQIzBAABAgAdFiEEJ6tQKp6olvZKJ0lwlcRvpqQRSKwFAl6bQXUACgkQlcRvpqQRSKyNhhAAkdPfbvlHIxpP3Euh28T0AlLRaqDy90JXuvTSzfXVZyu6amZKoF3zcFC1ZTUngXp7JrhY3lrI2zQoNH4skOBhn/va+mgSgoEwLXuP2hXW6vKjYSpYKYpFom9n35bHFfhLLeNk7dfYyG5f6CIu3Is8IrayvRtb9eoRnidIqxk9uPsvdtU/lkMpHE+XHYt6xA+IHaM+JFpCLZhrSZosIKLBSX0QBytq9LUXiAjYHs7xSov36v5P3R6CJfqc7koCXK/XNzNWFpyS4z0hxdm8bCcuoxrOyCeb2BtYTCD3qm/0pmSqfn8ubbgx/eC0EJn9J8E80aCejyn8bqsy+To0T+FZm2GJhOLrQfS6spte1K4ZIg8TBl8tlc6GtKH9Q0hwA8SaNEBTR4kWI6ImbotJ2co6N5eHB8v6uk33uCUVWQ+BA4ZeiltmQZ798lXUd3FP/lfaHm51HXgBMgGK1Ynxd+gcPqJpExjivImrT8jZC3b4VkGEp9FBBtX6F9d+V9S3ZrXWjmjZfO85inqXCaXK1YTWnajA/2AqLpxWMWroXH69//u5ZBkdqF76sqtjgMT0txSfe40Kl6DxIZyGKcVXPd4QpCRXWI/YG/x0qddKACr8yS3nO/TrFxXH4QUbexWA4vlCcxgaVr/Am9JDwOS+fe0yOx+zQ+JjMB0IoAOUR+5OTsE=
  account: mediocregopher

commit_change_test.go

@@ -5,11 +5,8 @@ import (
 	"strings"
 	"testing"
 
-	"dehub.dev/src/dehub.git/sigcred"
-
 	"github.com/davecgh/go-spew/spew"
 	"gopkg.in/src-d/go-git.v4/plumbing"
-	yaml "gopkg.in/yaml.v2"
 )
 
 func TestChangeCommitVerify(t *testing.T) {
@@ -76,10 +73,12 @@ func TestChangeCommitVerify(t *testing.T) {
 	for _, test := range testCases {
 		t.Run(test.descr, func(t *testing.T) {
 			h := newHarness(t)
+			rootSig := h.stageNewAccount("root", false)
+
 			for _, step := range test.steps {
 				h.stage(step.tree)
 
-				gitCommit := h.changeCommit(step.msg, h.sig)
+				gitCommit := h.assertCommitChange(true, step.msg, rootSig)
 				if step.msgHead == "" {
 					step.msgHead = strings.TrimSpace(step.msg) + "\n\n"
 				}
@@ -105,18 +104,13 @@ func TestCombineCommitChanges(t *testing.T) {
 	h := newHarness(t)
 
 	// commit initial config, so the root user can modify it in the next commit
-	h.changeCommit("initial commit", h.sig)
+	rootSig := h.stageNewAccount("root", false)
+	h.assertCommitChange(true, "initial commit", rootSig)
 
 	// add a toot user and modify the access controls such that both accounts
 	// are required for the main branch
-	tootSig, tootPubKeyBody := sigcred.TestSignifierPGP("toot", false, h.rand)
+	tootSig := h.stageNewAccount("toot", false)
-	h.cfg.Accounts = append(h.cfg.Accounts, Account{
+	h.stageAccessControls(`
-		ID: "toot",
-		Signifiers: []sigcred.Signifier{{PGPPublicKey: &sigcred.SignifierPGP{
-			Body: string(tootPubKeyBody),
-		}}},
-	})
-	err := yaml.Unmarshal([]byte(`
 - action: allow
   filters:
   - type: branch
@@ -136,22 +130,15 @@ func TestCombineCommitChanges(t *testing.T) {
   - type: signature
     any_account: true
     count: 1
-
+`)
-- action: deny
+	tootCommit := h.assertCommitChange(true, "add toot", rootSig)
-`), &h.cfg.AccessControls)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	h.stageCfg()
-	tootCommit := h.changeCommit("add toot", h.sig)
 
 	// make a single change commit in another branch using root. Then add a
 	// credential using toot, and combine them onto main.
 	otherBranch := plumbing.NewBranchReferenceName("other")
 	h.checkout(otherBranch)
 	h.stage(map[string]string{"foo": "bar"})
-	fooCommit := h.changeCommit("add foo file", h.sig)
+	fooCommit := h.assertCommitChange(true, "add foo file", rootSig)
 
 	// now adding a credential commit from toot should work
 	credCommitObj, err := h.repo.NewCommitCredential(fooCommit.Interface.GetHash())

commit_credential_test.go

@@ -3,28 +3,18 @@ package dehub
 import (
 	"testing"
 
-	"dehub.dev/src/dehub.git/sigcred"
-
 	"gopkg.in/src-d/go-git.v4/plumbing"
-	yaml "gopkg.in/yaml.v2"
 )
 
 func TestCredentialCommitVerify(t *testing.T) {
 	h := newHarness(t)
+	rootSig := h.stageNewAccount("root", false)
 
 	// create a new account and modify the config so that that account is only
 	// allowed to add verifications to a single branch
-	tootSig, tootPubKeyBody := sigcred.TestSignifierPGP("toot", false, h.rand)
+	tootSig := h.stageNewAccount("toot", false)
-	h.cfg.Accounts = append(h.cfg.Accounts, Account{
-		ID: "toot",
-		Signifiers: []sigcred.Signifier{{PGPPublicKey: &sigcred.SignifierPGP{
-			Body: string(tootPubKeyBody),
-		}}},
-	})
-
 	tootBranch := plumbing.NewBranchReferenceName("toot_branch")
-
+	h.stageAccessControls(`
-	err := yaml.Unmarshal([]byte(`
 - action: allow
   filters:
   - type: branch
@@ -41,14 +31,8 @@ func TestCredentialCommitVerify(t *testing.T) {
     count: 1
     account_ids:
     - root
-
+`)
-- action: deny
+	rootGitCommit := h.assertCommitChange(true, "initial commit", rootSig)
-`), &h.cfg.AccessControls)
-	if err != nil {
-		t.Fatal(err)
-	}
-	h.stageCfg()
-	rootGitCommit := h.changeCommit("initial commit", h.sig)
 
 	// toot user wants to create a credential commit for the root commit, for
 	// whatever reason.

commit_test.go

@@ -3,47 +3,34 @@ package dehub
 import (
 	"testing"
 
-	"dehub.dev/src/dehub.git/accessctl"
-	"dehub.dev/src/dehub.git/sigcred"
-
 	"gopkg.in/src-d/go-git.v4/plumbing"
 )
 
 func TestConfigChange(t *testing.T) {
 	h := newHarness(t)
+	rootSig := h.stageNewAccount("root", false)
 
 	var gitCommits []GitCommit
 
 	// commit the initial staged changes, which merely include the config and
 	// public key
-	gitCommit := h.changeCommit("commit configuration", h.sig)
+	gitCommit := h.assertCommitChange(true, "commit configuration", rootSig)
 	gitCommits = append(gitCommits, gitCommit)
 
 	// create a new account and add it to the configuration. That commit should
 	// not be verifiable, though
-	newSig, newPubKeyBody := sigcred.TestSignifierPGP("toot", false, h.rand)
+	tootSig := h.stageNewAccount("toot", false)
-	h.cfg.Accounts = append(h.cfg.Accounts, Account{
-		ID: "toot",
-		Signifiers: []sigcred.Signifier{{PGPPublicKey: &sigcred.SignifierPGP{
-			Body: string(newPubKeyBody),
-		}}},
-	})
-
 	h.stageCfg()
-	badCommit, err := h.repo.NewCommitChange("add toot user")
+	h.assertCommitChange(false, "add toot user", tootSig)
-	if err != nil {
-		t.Fatalf("creating CommitChange: %v", err)
-	}
-	h.tryCommit(false, badCommit, newSig)
 
 	// now add with the root user, this should work.
 	h.stageCfg()
-	gitCommit = h.changeCommit("add toot user", h.sig)
+	gitCommit = h.assertCommitChange(true, "add toot user", rootSig)
 	gitCommits = append(gitCommits, gitCommit)
 
 	// _now_ the toot user should be able to do things.
 	h.stage(map[string]string{"foo/bar": "what a cool file"})
-	gitCommit = h.changeCommit("add a cool file", newSig)
+	gitCommit = h.assertCommitChange(true, "add a cool file", tootSig)
 	gitCommits = append(gitCommits, gitCommit)
 
 	if err := h.repo.VerifyCommits(MainRefName, gitCommits); err != nil {
@@ -55,21 +42,19 @@ func TestMainAncestryRequirement(t *testing.T) {
 	otherBranch := plumbing.NewBranchReferenceName("other")
 	t.Run("empty repo", func(t *testing.T) {
 		h := newHarness(t)
+		rootSig := h.stageNewAccount("root", false)
 		h.checkout(otherBranch)
 
 		// stage and try to add to the "other" branch, it shouldn't work though
 		h.stageCfg()
-		badCommit, err := h.repo.NewCommitChange("starting new branch at other")
+		h.assertCommitChange(false, "starting new branch at other", rootSig)
-		if err != nil {
-			t.Fatalf("creating CommitChange: %v", err)
-		}
-		h.tryCommit(false, badCommit, h.sig)
 	})
 
 	t.Run("new branch, single commit", func(t *testing.T) {
 		h := newHarness(t)
+		rootSig := h.stageNewAccount("root", false)
 		h.stageCfg()
-		h.changeCommit("add cfg", h.sig)
+		h.assertCommitChange(true, "add cfg", rootSig)
 
 		// set HEAD to this other branch which doesn't really exist
 		ref := plumbing.NewSymbolicReference(plumbing.HEAD, otherBranch)
@@ -78,24 +63,19 @@ func TestMainAncestryRequirement(t *testing.T) {
 		}
 
 		h.stageCfg()
-		badCommit, err := h.repo.NewCommitChange("starting new branch at other")
+		h.assertCommitChange(false, "starting new branch at other", rootSig)
-		if err != nil {
-			t.Fatalf("creating CommitChange: %v", err)
-		}
-		h.tryCommit(false, badCommit, h.sig)
 	})
 }
 
 func TestAnonymousCommits(t *testing.T) {
 	h := newHarness(t)
-	anonSig, _ := sigcred.TestSignifierPGP("anon", true, h.rand)
+	anonSig := h.stageNewAccount("anon", true)
 
-	h.cfg.AccessControls = []accessctl.AccessControl{{
+	h.stageAccessControls(`
-		Action: accessctl.ActionAllow,
+- action: allow
-		Filters: []accessctl.Filter{
+  filters:
-			{Signature: &accessctl.FilterSignature{Any: true}},
+    - type: signature
-		},
+      any: true
-	}}
+`)
-	h.stageCfg()
+	h.assertCommitChange(true, "this will work", anonSig)
-	h.changeCommit("this will work", anonSig)
 }

repo_test.go

@@ -6,7 +6,6 @@ import (
 	"io"
 	"math/rand"
 	"path/filepath"
-	"runtime/debug"
 	"testing"
 
 	"dehub.dev/src/dehub.git/sigcred"
@@ -21,40 +20,16 @@ type harness struct {
 	rand *rand.Rand
 	repo *Repo
 	cfg  *Config
-	sig  sigcred.SignifierInterface
 }
 
 func newHarness(t *testing.T) *harness {
 	rand := rand.New(rand.NewSource(0xb4eadb01))
-	sig, pubKeyBody := sigcred.TestSignifierPGP("root", false, rand)
+	return &harness{
-	pubKeyPath := filepath.Join(DehubDir, "root.asc")
-
-	cfg := &Config{
-		Accounts: []Account{{
-			ID: "root",
-			Signifiers: []sigcred.Signifier{{PGPPublicKeyFile: &sigcred.SignifierPGPFile{
-				Path: pubKeyPath,
-			}}},
-		}},
-	}
-	cfgBody, err := yaml.Marshal(cfg)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	h := &harness{
 		t:    t,
 		rand: rand,
 		repo: InitMemRepo(),
-		cfg:  cfg,
+		cfg:  new(Config),
-		sig:  sig,
 	}
-	h.stage(map[string]string{
-		ConfigPath: string(cfgBody),
-		pubKeyPath: string(pubKeyBody),
-	})
-
-	return h
 }
 
 func (h *harness) stage(tree map[string]string) {
@@ -100,6 +75,27 @@ func (h *harness) stageCfg() {
 	h.stage(map[string]string{ConfigPath: string(cfgBody)})
 }
 
+func (h *harness) stageNewAccount(accountID string, anon bool) sigcred.SignifierInterface {
+	sig, pubKeyBody := sigcred.TestSignifierPGP(accountID, anon, h.rand)
+	if !anon {
+		h.cfg.Accounts = append(h.cfg.Accounts, Account{
+			ID: accountID,
+			Signifiers: []sigcred.Signifier{{PGPPublicKey: &sigcred.SignifierPGP{
+				Body: string(pubKeyBody),
+			}}},
+		})
+		h.stageCfg()
+	}
+	return sig
+}
+
+func (h *harness) stageAccessControls(aclYAML string) {
+	if err := yaml.Unmarshal([]byte(aclYAML), &h.cfg.AccessControls); err != nil {
+		h.t.Fatal(err)
+	}
+	h.stageCfg()
+}
+
 func (h *harness) checkout(branch plumbing.ReferenceName) {
 
 	w, err := h.repo.GitRepo.Worktree()
@@ -195,7 +191,8 @@ func (h *harness) tryCommit(
 	return gitCommit
 }
 
-func (h *harness) changeCommit(
+func (h *harness) assertCommitChange(
+	shouldSucceed bool,
 	msg string,
 	sig sigcred.SignifierInterface,
 ) GitCommit {
@@ -203,18 +200,17 @@ func (h *harness) changeCommit(
 	if err != nil {
 		h.t.Fatalf("creating ChangeCommit: %v", err)
 	}
-	return h.tryCommit(true, commit, sig)
+	return h.tryCommit(shouldSucceed, commit, sig)
 }
 
 func TestHasStagedChanges(t *testing.T) {
-	harness := newHarness(t)
+	h := newHarness(t)
+	rootSig := h.stageNewAccount("root", false)
 	assertHasStaged := func(expHasStaged bool) {
-		hasStaged, err := harness.repo.HasStagedChanges()
+		hasStaged, err := h.repo.HasStagedChanges()
 		if err != nil {
-			debug.PrintStack()
 			t.Fatalf("error calling HasStagedChanges: %v", err)
 		} else if hasStaged != expHasStaged {
-			debug.PrintStack()
 			t.Fatalf("expected HasStagedChanges to return %v", expHasStaged)
 		}
 	}
@@ -222,64 +218,17 @@ func TestHasStagedChanges(t *testing.T) {
 	// the harness starts with some staged changes
 	assertHasStaged(true)
 
-	harness.stage(map[string]string{"foo": "bar"})
+	h.stage(map[string]string{"foo": "bar"})
 	assertHasStaged(true)
-	harness.changeCommit("first commit", harness.sig)
+	h.assertCommitChange(true, "first commit", rootSig)
 	assertHasStaged(false)
 
-	harness.stage(map[string]string{"foo": ""}) // delete foo
+	h.stage(map[string]string{"foo": ""}) // delete foo
 	assertHasStaged(true)
-	harness.changeCommit("second commit", harness.sig)
+	h.assertCommitChange(true, "second commit", rootSig)
 	assertHasStaged(false)
 }
 
-// TestOldConfig tests that having an older, now malformed, Config doesn't mess
-// with the current parsing, as long as the default access controls still work.
-func TestOldConfig(t *testing.T) {
-	harness := newHarness(t)
-
-	// overwrite the currently staged config file with an older form
-	harness.stage(map[string]string{ConfigPath: `
----
-accounts:
-  - id: root
-    signifiers:
-      - type: pgp_public_key_file
-        path: ".dehub/root.asc"
-
-access_controls:
-  - pattern: "**"
-    condition:
-      type: signature
-      account_ids:
-        - root
-      count: 0
-`})
-
-	// this commit should be created and verify fine
-	harness.changeCommit("first commit, this is going great", harness.sig)
-
-	// this commit should not be verifiable, because toot isn't in accounts and
-	// the default access controls should be being used
-	harness.stage(map[string]string{"foo": "no rules!"})
-	badCommit, err := harness.repo.NewCommitChange("ain't no laws")
-	if err != nil {
-		t.Fatalf("creating CommitChange: %v", err)
-	}
-	harness.tryCommit(false, badCommit, nil)
-
-	// make a commit fixing the config. everything should still be fine.
-	harness.stage(map[string]string{ConfigPath: `
----
-accounts:
-  - id: root
-    signifiers:
-      - type: pgp_public_key_file
-        path: ".dehub/root.asc"
-`})
-	harness.changeCommit("Fix the config!", harness.sig)
-}
-
 // TestThisRepoStillVerifies opens this actual repository and ensures that all
 // commits in it still verify, given this codebase.
 func TestThisRepoStillVerifies(t *testing.T) {
@@ -310,10 +259,11 @@ func TestThisRepoStillVerifies(t *testing.T) {
 }
 
 func TestShortHashResolving(t *testing.T) {
-	// TODO ideally this test would test the conflicting hashes are noticed, but
+	// TODO ideally this test would test that conflicting hashes are noticed,
-	// that's hard...
+	// but that's hard...
 	h := newHarness(t)
-	hash := h.changeCommit("first commit", h.sig).GitCommit.Hash
+	rootSig := h.stageNewAccount("root", false)
+	hash := h.assertCommitChange(true, "first commit", rootSig).GitCommit.Hash
 	hashStr := hash.String()
 	t.Log(hashStr)