pruning: fix tests + implement 'not_replicated' and 'keep_regex' keep rule

tests expected that a KeepRule returns a *keep* list whereas it
actually returns a *destroy* list.

cmd/config_parse.go

@@ -188,7 +188,7 @@ func parseKeepRule(in config.PruningEnum) (p PrunePolicy, err error) {
 	case config.PruneGrid:
 		return retentiongrid.ParseGridPrunePolicy(v, willSeeBookmarks)
 	//case config.PruneKeepLastN:
-	//case config.PruneKeepPrefix:
+	//case config.PruneKeepRegex:
 	//case config.PruneKeepNotReplicated:
 	default:
 		panic(fmt.Sprintf("unknown keep rule type %v", v))

config/config.go

@@ -179,9 +179,9 @@ type PruneKeepLastN struct {
 	Count int    `yaml:"count"`
 }
 
-type PruneKeepPrefix struct { // FIXME rename to KeepPrefix
+type PruneKeepRegex struct { // FIXME rename to KeepRegex
-	Type   string `yaml:"type"`
+	Type  string `yaml:"type"`
-	Prefix string `yaml:"prefix"`
+	Regex string `yaml:"prefix"`
 }
 
 type LoggingOutletEnum struct {
@@ -301,7 +301,7 @@ func (t *PruningEnum) UnmarshalYAML(u func(interface{}, bool) error) (err error)
 		"not_replicated": &PruneKeepNotReplicated{},
 		"last_n":         &PruneKeepLastN{},
 		"grid":           &PruneGrid{},
-		"prefix":         &PruneKeepPrefix{},
+		"prefix":         &PruneKeepRegex{},
 	})
 	return
 }

pruning/keep_last_n.go

@@ -16,10 +16,10 @@ func NewKeepLastN(n int) (*KeepLastN, error) {
 	return &KeepLastN{n}, nil
 }
 
-func (k KeepLastN) KeepRule(snaps []Snapshot) []Snapshot {
+func (k KeepLastN) KeepRule(snaps []Snapshot) (destroyList []Snapshot) {
 
 	if k.n > len(snaps) {
-		return snaps
+		return []Snapshot{}
 	}
 
 	res := shallowCopySnapList(snaps)
@@ -28,5 +28,5 @@ func (k KeepLastN) KeepRule(snaps []Snapshot) []Snapshot {
 		return res[i].Date().After(res[j].Date())
 	})
 
-	return res[:k.n]
+	return res[k.n:]
 }

pruning/keep_last_n_test.go

@@ -29,17 +29,18 @@ func TestKeepLastN(t *testing.T) {
 			rules: []KeepRule{
 				KeepLastN{2},
 			},
-			exp: map[string]bool{
+			expDestroy: map[string]bool{
-				"4": true, "5": true,
+				"1": true, "2": true, "3": true,
 			},
 		},
-		"keep1": { // Keep one of two with same time
+		"keep1OfTwoWithSameTime": { // Keep one of two with same time
 			inputs: inputs["s1"],
 			rules: []KeepRule{
 				KeepLastN{1},
 			},
-			exp: map[string]bool{
+			expDestroyAlternatives: []map[string]bool{
-				"4": true, //5 would be ok too
+				{"1": true, "2": true, "3": true, "4": true},
+				{"1": true, "2": true, "3": true, "5": true},
 			},
 		},
 		"keepMany": {
@@ -47,20 +48,14 @@ func TestKeepLastN(t *testing.T) {
 			rules: []KeepRule{
 				KeepLastN{100},
 			},
-			exp: map[string]bool{
+			expDestroy: map[string]bool{},
-				"1": true,
-				"2": true,
-				"3": true,
-				"4": true,
-				"5": true,
-			},
 		},
 		"empty": {
 			inputs: inputs["s2"],
 			rules: []KeepRule{
 				KeepLastN{100},
 			},
-			exp: map[string]bool{},
+			expDestroy: map[string]bool{},
 		},
 	}
 

pruning/keep_not_replicated.go

@@ -0,0 +1,15 @@
+package pruning
+
+type KeepNotReplicated struct {
+	forceConstructor struct{}
+}
+
+func (*KeepNotReplicated) KeepRule(snaps []Snapshot) (destroyList []Snapshot) {
+	return filterSnapList(snaps, func(snapshot Snapshot) bool {
+		return snapshot.Replicated()
+	})
+}
+
+func NewKeepNotReplicated() *KeepNotReplicated {
+	return &KeepNotReplicated{}
+}

pruning/keep_not_replicated_test.go

@@ -0,0 +1,39 @@
+package pruning
+
+import (
+	"testing"
+)
+
+func TestNewKeepNotReplicated(t *testing.T) {
+
+	inputs := map[string][]Snapshot{
+		"s1": []Snapshot{
+			stubSnap{name: "1", replicated: true},
+			stubSnap{name: "2", replicated: false},
+			stubSnap{name: "3", replicated: true},
+		},
+		"s2": []Snapshot{},
+	}
+
+	tcs := map[string]testCase{
+		"destroysOnlyReplicated": {
+			inputs: inputs["s1"],
+			rules: []KeepRule{
+				NewKeepNotReplicated(),
+			},
+			expDestroy: map[string]bool{
+				"1": true, "3": true,
+			},
+		},
+		"empty": {
+			inputs: inputs["s2"],
+			rules: []KeepRule{
+				NewKeepNotReplicated(),
+			},
+			expDestroy: map[string]bool{},
+		},
+	}
+
+	testTable(tcs, t)
+
+}

pruning/pruning.go

@@ -1,11 +1,14 @@
 package pruning
 
 import (
+	"fmt"
+	"github.com/pkg/errors"
+	"github.com/zrepl/zrepl/config"
 	"time"
 )
 
 type KeepRule interface {
-	KeepRule(snaps []Snapshot) []Snapshot
+	KeepRule(snaps []Snapshot) (destroyList []Snapshot)
 }
 
 type Snapshot interface {
@@ -17,8 +20,8 @@ type Snapshot interface {
 // The returned snapshot list is guaranteed to only contains elements of input parameter snaps
 func PruneSnapshots(snaps []Snapshot, keepRules []KeepRule) []Snapshot {
 
-	if len(keepRules) == 0 {
+	if keepRules == nil || len(keepRules) == 0 {
-		return snaps
+		return []Snapshot{}
 	}
 
 	remCount := make(map[Snapshot]int, len(snaps))
@@ -38,3 +41,27 @@ func PruneSnapshots(snaps []Snapshot, keepRules []KeepRule) []Snapshot {
 
 	return remove
 }
+
+func RulesFromConfig(in []config.PruningEnum) (rules []KeepRule, err error) {
+	rules = make([]KeepRule, len(in))
+	for i := range in {
+		rules[i], err = RuleFromConfig(in[i])
+		if err != nil {
+			return nil, errors.Wrapf(err, "cannot build rule #%d", i)
+		}
+	}
+	return rules, nil
+}
+
+func RuleFromConfig(in config.PruningEnum) (KeepRule, error) {
+	switch v := in.Ret.(type) {
+	case *config.PruneKeepNotReplicated:
+		return NewKeepNotReplicated(), nil
+	case *config.PruneKeepLastN:
+		return NewKeepLastN(v.Count)
+	case *config.PruneKeepRegex:
+		return NewKeepRegex(v.Regex)
+	default:
+		return nil, fmt.Errorf("unknown keep rule type %T", v)
+	}
+}

pruning/pruning_test.go

@@ -1,8 +1,6 @@
 package pruning
 
 import (
-	"fmt"
-	"github.com/stretchr/testify/assert"
 	"testing"
 	"time"
 )
@@ -20,9 +18,10 @@ func (s stubSnap) Replicated() bool { return s.replicated }
 func (s stubSnap) Date() time.Time { return s.date }
 
 type testCase struct {
-	inputs   []Snapshot
+	inputs                 []Snapshot
-	rules    []KeepRule
+	rules                  []KeepRule
-	exp, eff map[string]bool
+	expDestroy, effDestroy map[string]bool
+	expDestroyAlternatives []map[string]bool
 }
 
 func testTable(tcs map[string]testCase, t *testing.T) {
@@ -42,11 +41,26 @@ func testTable(tcs map[string]testCase, t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			tc := tcs[name]
 			remove := PruneSnapshots(tc.inputs, tc.rules)
-			tc.eff = make(map[string]bool)
+			tc.effDestroy = make(map[string]bool)
 			for _, s := range remove {
-				tc.eff[s.Name()] = true
+				tc.effDestroy[s.Name()] = true
+			}
+			if tc.expDestroyAlternatives == nil {
+				if tc.expDestroy == nil {
+					panic("must specify either expDestroyAlternatives or expDestroy")
+				}
+				tc.expDestroyAlternatives = []map[string]bool{tc.expDestroy}
+			}
+			var okAlt map[string]bool = nil
+			for _, alt := range tc.expDestroyAlternatives {
+				t.Logf("testing possible result: %v", alt)
+				if mapEqual(alt, tc.effDestroy) {
+					okAlt = alt
+				}
+			}
+			if okAlt == nil {
+				t.Errorf("no alternatives matched result: %v", tc.effDestroy)
 			}
-			assert.True(t, mapEqual(tc.exp, tc.eff), fmt.Sprintf("is %v but should be %v", tc.eff, tc.exp))
 		})
 	}
 }
@@ -67,7 +81,7 @@ func TestPruneSnapshots(t *testing.T) {
 			rules: []KeepRule{
 				MustKeepRegex("foo_"),
 			},
-			exp: map[string]bool{
+			expDestroy: map[string]bool{
 				"bar_123": true,
 			},
 		},
@@ -77,7 +91,7 @@ func TestPruneSnapshots(t *testing.T) {
 				MustKeepRegex("foo_"),
 				MustKeepRegex("bar_"),
 			},
-			exp: map[string]bool{},
+			expDestroy: map[string]bool{},
 		},
 		"onlyThoseRemovedByAllAreRemoved": {
 			inputs: inputs["s1"],
@@ -85,26 +99,27 @@ func TestPruneSnapshots(t *testing.T) {
 				MustKeepRegex("notInS1"), // would remove all
 				MustKeepRegex("bar_"),    // would remove all but bar_, i.e. foo_.*
 			},
-			exp: map[string]bool{
+			expDestroy: map[string]bool{
 				"foo_123": true,
 				"foo_456": true,
 			},
 		},
 		"noRulesKeepsAll": {
-			inputs: inputs["s1"],
+			inputs:     inputs["s1"],
-			rules:  []KeepRule{},
+			rules:      []KeepRule{},
-			exp: map[string]bool{
+			expDestroy: map[string]bool{},
-				"foo_123": true,
+		},
-				"foo_456": true,
+		"nilRulesKeepsAll": {
-				"bar_123": true,
+			inputs:     inputs["s1"],
-			},
+			rules:      nil,
+			expDestroy: map[string]bool{},
 		},
 		"noSnaps": {
 			inputs: []Snapshot{},
 			rules: []KeepRule{
 				MustKeepRegex("foo_"),
 			},
-			exp: map[string]bool{},
+			expDestroy: map[string]bool{},
 		},
 	}