ActionFinish refactor fix commit

This commit wraps up the changes to `action-set` now that ActionFinish
has landed in core.

worker/uniter/context.go

@@ -155,10 +155,6 @@ func NewHookContext(
 		return nil, err
 	}
 
-	ctx.actionResults = actionResults{
-		Results: map[string]interface{}{},
-		Status:  actionStatusInit,
-	}
 	return ctx, nil
 }
 
@@ -226,7 +222,7 @@ func (ctx *HookContext) SetActionFailed(message string) error {
 // Action-containing HookContext.
 func (ctx *HookContext) UpdateActionResults(keys []string, value string) error {
 	if ctx.actionData == nil {
-		return fmt.Errorf("action results cannot be updated, hook context had no action")
+		return fmt.Errorf("not running an action")
 	}
 	addValueToMap(keys, value, ctx.actionData.ResultsMap)
 	return nil
@@ -757,20 +753,13 @@ func newActionData(tag *names.ActionTag, params map[string]interface{}) *actionD
 	}
 }
 
-// actionResults contains the results of an action, and any response message.
-type actionResults struct {
-	Message string
-	Status  string
-	Results map[string]interface{}
-}
-
 // actionStatus messages define the possible states of a completed Action.
 const (
 	actionStatusInit   = "init"
 	actionStatusFailed = "fail"
 )
 
-// AddValueToMap adds the given value to the map on which the method is run.
+// addValueToMap adds the given value to the map on which the method is run.
 // This allows us to merge maps such as {foo: {bar: baz}} and {foo: {baz: faz}}
 // into {foo: {bar: baz, baz: faz}}.
 func addValueToMap(keys []string, value string, target map[string]interface{}) {

worker/uniter/context_test.go

@@ -4,7 +4,6 @@
 package uniter_test
 
 import (
-	"bytes"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -13,7 +12,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/juju/cmd"
 	"github.com/juju/names"
 	envtesting "github.com/juju/testing"
 	jc "github.com/juju/testing/checkers"
@@ -28,7 +26,6 @@ import (
 	"github.com/juju/juju/juju/testing"
 	"github.com/juju/juju/network"
 	"github.com/juju/juju/state"
-	contexttesting "github.com/juju/juju/testing"
 	"github.com/juju/juju/version"
 	"github.com/juju/juju/worker/uniter"
 	"github.com/juju/juju/worker/uniter/jujuc"
@@ -776,16 +773,6 @@ func (s *HookContextSuite) AddUnit(c *gc.C, svc *state.Service) *state.Unit {
 	return unit
 }
 
-func (s *HookContextSuite) TestNonActionCallsToActionMethodsFail(c *gc.C) {
-	ctx := uniter.HookContext{}
-	_, err := ctx.ActionParams()
-	c.Check(err, gc.ErrorMatches, "not running an action")
-	err = ctx.SetActionFailed("oops")
-	c.Check(err, gc.ErrorMatches, "not running an action")
-	err = ctx.RunAction("asdf", "fdsa", "qwerty", "uiop")
-	c.Check(err, gc.ErrorMatches, "not running an action")
-}
-
 func (s *HookContextSuite) AddContextRelation(c *gc.C, name string) {
 	s.AddTestingService(c, name, s.relch)
 	eps, err := s.State.InferEndpoints("u", name)
@@ -826,49 +813,59 @@ func (s *HookContextSuite) getHookContext(c *gc.C, uuid string, relid int,
 func (s *HookContextSuite) TestNonActionCallsToActionMethodsFail(c *gc.C) {
 	ctx := uniter.HookContext{}
 	_, err := ctx.ActionParams()
-	c.Check(err, gc.ErrorMatches, "actionparams cannot be retrieved, hook context had no action")
+	c.Check(err, gc.ErrorMatches, "not running an action")
 	err = ctx.SetActionFailed("oops")
-	c.Check(err, gc.ErrorMatches, "action cannot be failed, hook context had no action")
-	err = ctx.UpdateActionResults([]string{"oops"}, "nope!")
-	c.Check(err, gc.ErrorMatches, "action results cannot be updated, hook context had no action")
+	c.Check(err, gc.ErrorMatches, "not running an action")
+	err = ctx.RunAction("asdf", "fdsa", "qwerty", "uiop")
+	c.Check(err, gc.ErrorMatches, "not running an action")
+	err = ctx.UpdateActionResults([]string{"1", "2", "3"}, "value")
+	c.Check(err, gc.ErrorMatches, "not running an action")
 }
 
 // TestUpdateActionResults demonstrates that UpdateActionResults functions
-// as expected; the further tests of action-set are in jujuc/action-set_test.
+// as expected.
 func (s *HookContextSuite) TestUpdateActionResults(c *gc.C) {
 	tests := []struct {
+		initial  map[string]interface{}
+		keys     []string
+		value    string
 		expected map[string]interface{}
-		errMsg   string
-		code     int
-		command  []string
 	}{{
-		command: []string{"foo.bar=baz", "foo.baz=bar",
-			"bar.foo=foo2", "bar=5"},
+		initial: map[string]interface{}{},
+		keys:    []string{"foo"},
+		value:   "bar",
+		expected: map[string]interface{}{
+			"foo": "bar",
+		},
+	}, {
+		initial: map[string]interface{}{
+			"foo": "bar",
+		},
+		keys:  []string{"foo", "bar"},
+		value: "baz",
 		expected: map[string]interface{}{
 			"foo": map[string]interface{}{
 				"bar": "baz",
-				"baz": "bar",
 			},
-			"bar": "5",
 		},
 	}, {
-		command:  []string{"foo-=5"},
-		expected: map[string]interface{}{},
-		errMsg: "error: key \"foo-\" must start and end with " +
-			"lowercase alphanumeric, and contain only " +
-			"lowercase alphanumeric and hyphens\n",
-		code: 2,
+		initial: map[string]interface{}{
+			"foo": map[string]interface{}{
+				"bar": "baz",
+			},
+		},
+		keys:  []string{"foo"},
+		value: "bar",
+		expected: map[string]interface{}{
+			"foo": "bar",
+		},
 	}}
 
 	for i, t := range tests {
-		c.Logf("action-set test %d: %#v", i, t.command)
-		hctx := uniter.GetStubActionContext()
-		com, err := jujuc.NewCommand(hctx, "action-set")
+		c.Logf("UpdateActionResults test %d: %#v: %#v", i, t.keys, t.value)
+		hctx := uniter.GetStubActionContext(t.initial)
+		err := hctx.UpdateActionResults(t.keys, t.value)
 		c.Assert(err, gc.IsNil)
-		ctx := contexttesting.Context(c)
-		code := cmd.Main(com, ctx, t.command)
-		c.Check(code, gc.Equals, t.code)
-		c.Check(ctx.Stderr.(*bytes.Buffer).String(), gc.Equals, t.errMsg)
 		c.Check(hctx.ActionResultsMap(), jc.DeepEquals, t.expected)
 	}
 }

worker/uniter/export_test.go

@@ -18,21 +18,30 @@ func (u *Uniter) GetProxyValues() proxy.Settings {
 }
 
 func (c *HookContext) ActionResultsMap() map[string]interface{} {
+	if c.actionData == nil {
+		panic("context not running an action")
+	}
 	return c.actionData.ResultsMap
 }
 
 func (c *HookContext) ActionFailed() bool {
+	if c.actionData == nil {
+		panic("context not running an action")
+	}
 	return c.actionData.ActionFailed
 }
 
 func (c *HookContext) ActionMessage() string {
+	if c.actionData == nil {
+		panic("context not running an action")
+	}
 	return c.actionData.ResultsMessage
 }
 
-func GetStubActionContext() *HookContext {
+func GetStubActionContext(in map[string]interface{}) *HookContext {
 	return &HookContext{
 		actionData: &actionData{
-			ResultsMap: map[string]interface{}{},
+			ResultsMap: in,
 		},
 	}
 }

worker/uniter/jujuc/action-set_test.go

@@ -8,7 +8,7 @@ import (
 
 	"github.com/juju/cmd"
 	jc "github.com/juju/testing/checkers"
-	gc "launchpad.net/gocheck"
+	gc "gopkg.in/check.v1"
 
 	"github.com/juju/juju/testing"
 	"github.com/juju/juju/worker/uniter/jujuc"
@@ -39,7 +39,7 @@ type nonActionSettingContext struct {
 }
 
 func (a *nonActionSettingContext) UpdateActionResults(keys []string, value string) error {
-	return fmt.Errorf("cannot update results on a context that doesn't contain actionData")
+	return fmt.Errorf("not running an action")
 }
 
 func (s *ActionSetSuite) TestActionSetOnNonActionContextFails(c *gc.C) {
@@ -50,7 +50,7 @@ func (s *ActionSetSuite) TestActionSetOnNonActionContextFails(c *gc.C) {
 	code := cmd.Main(com, ctx, []string{"oops=nope"})
 	c.Check(code, gc.Equals, 1)
 	c.Check(bufferString(ctx.Stdout), gc.Equals, "")
-	expect := fmt.Sprintf(`(.|\n)*error: %s\n`, "cannot update results on a context that doesn't contain actionData")
+	expect := fmt.Sprintf(`(\n)*error: %s\n`, "not running an action")
 	c.Check(bufferString(ctx.Stderr), gc.Matches, expect)
 }
 

worker/uniter/jujuc/util_test.go

@@ -134,7 +134,8 @@ func (c *Context) ActionParams() (map[string]interface{}, error) {
 	return c.actionParams, nil
 }
 
-func (c *Context) UpdateActionResults(keys []string, value string) {
+func (c *Context) UpdateActionResults(keys []string, value string) error {
+	return fmt.Errorf("not running an action")
 }
 
 func (c *Context) HookRelation() (jujuc.ContextRelation, bool) {

worker/uniter/uniter_test.go

@@ -1315,8 +1315,9 @@ var actionEventTests = []uniterTest{
 		addAction{"action-log", nil},
 		waitHooks{"action-log"},
 		verifyActionResults{[]actionResult{{
-			name:   "action-log",
-			status: "complete",
+			name:    "action-log",
+			results: map[string]interface{}{},
+			status:  "complete",
 		}}},
 		waitUnit{status: params.StatusStarted},
 	), ut(