Merge lp:~elopio/snappy/return-string into lp:~fgimenez/snappy/go-functional-tests

[Leo Arias]

Return a string on exec.

If the error we log is not converted to string, it will just print bytes. And if we convert it to string on the execCommand function, then it makes sense to return that too.

Also, merged with trunk.

[Federico Gimenez (fgimenez)]

There are places where we don't use the returned value from the execCommand function, in that cases maybe the conversion is not needed. Maybe we could add another function that calls execComand and returns the converted value?

Thanks!

[Federico Gimenez (fgimenez)]

Approve, we can benchmark the alternative later

review: Approve

=== modified file '_integration-tests/tests/snappy_test.go'
--- _integration-tests/tests/snappy_test.go	2015-06-19 04:26:01 +0000
+++ _integration-tests/tests/snappy_test.go	2015-06-22 23:53:50 +0000
@@ -33,15 +33,16 @@
 
 type InstallSuite struct{}
 
-func (s *InstallSuite) installSnap(c *C, packageName string) []byte {
+func (s *InstallSuite) installSnap(c *C, packageName string) string {
 	return s.execCommand(c, "sudo", "snappy", "install", packageName)
 }
 
-func (s *InstallSuite) execCommand(c *C, cmds ...string) []byte {
+func (s *InstallSuite) execCommand(c *C, cmds ...string) string {
 	cmd := exec.Command(cmds[0], cmds[1:len(cmds)]...)
 	output, err := cmd.CombinedOutput()
-	c.Assert(err, IsNil, Commentf("Error: %v", output))
-	return output
+	stringOutput := string(output)
+	c.Assert(err, IsNil, Commentf("Error: %v", stringOutput))
+	return stringOutput
 }
 
 func (s *InstallSuite) SetUpSuite(c *C) {
@@ -61,7 +62,7 @@
 		".*\n" +
 		"hello-world   .* .*  canonical \n" +
 		".*\n"
-	c.Assert(string(installOutput), Matches, expected)
+	c.Assert(installOutput, Matches, expected)
 }
 
 func (s *InstallSuite) TestCallBinaryFromInstalledSnap(c *C) {
@@ -69,7 +70,7 @@
 
 	echoOutput := s.execCommand(c, "hello-world.echo")
 
-	c.Assert(string(echoOutput), Equals, "Hello World!\n")
+	c.Assert(echoOutput, Equals, "Hello World!\n")
 }
 
 func (s *InstallSuite) TestInfoMustPrintInstalledPackageInformation(c *C) {
@@ -78,5 +79,5 @@
 	infoOutput := s.execCommand(c, "sudo", "snappy", "info")
 
 	expected := "(?ms).*^apps: hello-world\n"
-	c.Assert(string(infoOutput), Matches, expected)
+	c.Assert(infoOutput, Matches, expected)
 }

=== modified file 'provisioning/provisioning.go'
--- provisioning/provisioning.go	2015-06-09 18:07:51 +0000
+++ provisioning/provisioning.go	2015-06-22 23:53:50 +0000
@@ -29,18 +29,24 @@
 	"gopkg.in/yaml.v2"
 )
 
-var (
+const (
 	// InstallYamlFile is the name of the file created by
 	// ubuntu-device-flash(1), created at system installation time,
 	// that contains metadata on the installation.
 	//
 	// XXX: Public for ubuntu-device-flash(1)
 	InstallYamlFile = "install.yaml"
-
-	// ErrNoInstallYaml is emitted when InstallYamlFile does not exist.
-	ErrNoInstallYaml = fmt.Errorf("no %s", InstallYamlFile)
 )
 
+// ErrNoInstallYaml is emitted when InstallYamlFile does not exist.
+type ErrNoInstallYaml struct {
+	origErr error
+}
+
+func (e *ErrNoInstallYaml) Error() string {
+	return fmt.Sprintf("failed to read provisioning data: %s", e.origErr)
+}
+
 // InstallMeta encapsulates the metadata for a system install.
 type InstallMeta struct {
 	Timestamp         time.Time
@@ -79,7 +85,7 @@
 func parseInstallYaml(path string) (*InstallYaml, error) {
 	data, err := ioutil.ReadFile(path)
 	if err != nil {
-		return nil, ErrNoInstallYaml
+		return nil, &ErrNoInstallYaml{origErr: err}
 	}
 
 	return parseInstallYamlData(data)
@@ -124,3 +130,21 @@
 
 	return false
 }
+
+// InDeveloperMode returns true if the image was build with --developer-mode
+func InDeveloperMode(bootloaderDir string) bool {
+	file := filepath.Join(bootloaderDir, InstallYamlFile)
+
+	if !helpers.FileExists(file) {
+		// no idea
+		return false
+	}
+
+	InstallYaml, err := parseInstallYaml(file)
+	if err != nil {
+		// no idea
+		return false
+	}
+
+	return InstallYaml.InstallOptions.DeveloperMode
+}

=== modified file 'provisioning/provisioning_test.go'
--- provisioning/provisioning_test.go	2015-06-10 12:52:22 +0000
+++ provisioning/provisioning_test.go	2015-06-22 23:53:50 +0000
@@ -30,8 +30,8 @@
 func Test(t *testing.T) { TestingT(t) }
 
 type ProvisioningTestSuite struct {
-	tempdir         string
-	installYamlFile string
+	mockBootDir  string
+	mockYamlFile string
 }
 
 var _ = Suite(&ProvisioningTestSuite{})
@@ -78,74 +78,70 @@
 var garbageData = `Fooled you!?`
 
 func (ts *ProvisioningTestSuite) SetUpTest(c *C) {
-	ts.tempdir = c.MkDir()
-
-	ts.installYamlFile = filepath.Join(ts.tempdir, "install.yaml")
-
-	InstallYamlFile = ts.installYamlFile
-	os.Remove(InstallYamlFile)
+	ts.mockBootDir = c.MkDir()
+	ts.mockYamlFile = filepath.Join(ts.mockBootDir, "install.yaml")
 }
 
 func (ts *ProvisioningTestSuite) TestSideLoadedSystemNoInstallYaml(c *C) {
-	c.Assert(IsSideLoaded(""), Equals, false)
+	c.Assert(IsSideLoaded(ts.mockBootDir), Equals, false)
 }
 
 func (ts *ProvisioningTestSuite) TestSideLoadedSystem(c *C) {
-	c.Assert(IsSideLoaded(""), Equals, false)
+	c.Assert(IsSideLoaded(ts.mockBootDir), Equals, false)
 
-	err := ioutil.WriteFile(InstallYamlFile, []byte(yamlData), 0750)
+	err := ioutil.WriteFile(ts.mockYamlFile, []byte(yamlData), 0750)
 	c.Assert(err, IsNil)
 
-	c.Assert(IsSideLoaded(""), Equals, true)
+	c.Assert(IsSideLoaded(ts.mockBootDir), Equals, true)
 
-	os.Remove(InstallYamlFile)
-	c.Assert(IsSideLoaded(""), Equals, false)
+	os.Remove(ts.mockYamlFile)
+	c.Assert(IsSideLoaded(ts.mockBootDir), Equals, false)
 }
 
 func (ts *ProvisioningTestSuite) TestSideLoadedSystemNoDevicePart(c *C) {
 
-	c.Assert(IsSideLoaded(""), Equals, false)
+	c.Assert(IsSideLoaded(ts.mockBootDir), Equals, false)
 
-	err := ioutil.WriteFile(InstallYamlFile, []byte(yamlDataNoDevicePart), 0750)
+	err := ioutil.WriteFile(ts.mockYamlFile, []byte(yamlDataNoDevicePart), 0750)
 	c.Assert(err, IsNil)
 
-	c.Assert(IsSideLoaded(""), Equals, false)
+	c.Assert(IsSideLoaded(ts.mockBootDir), Equals, false)
 
-	os.Remove(InstallYamlFile)
-	c.Assert(IsSideLoaded(""), Equals, false)
+	os.Remove(ts.mockYamlFile)
+	c.Assert(IsSideLoaded(ts.mockBootDir), Equals, false)
 }
 
 func (ts *ProvisioningTestSuite) TestSideLoadedSystemGarbageInstallYaml(c *C) {
-	c.Assert(IsSideLoaded(""), Equals, false)
+	c.Assert(IsSideLoaded(ts.mockBootDir), Equals, false)
 
-	err := ioutil.WriteFile(InstallYamlFile, []byte(garbageData), 0750)
+	err := ioutil.WriteFile(ts.mockYamlFile, []byte(garbageData), 0750)
 	c.Assert(err, IsNil)
 
 	// we assume sideloaded if the file isn't parseable
-	c.Assert(IsSideLoaded(""), Equals, true)
+	c.Assert(IsSideLoaded(ts.mockBootDir), Equals, true)
 
-	os.Remove(InstallYamlFile)
-	c.Assert(IsSideLoaded(""), Equals, false)
+	os.Remove(ts.mockYamlFile)
+	c.Assert(IsSideLoaded(ts.mockBootDir), Equals, false)
 }
 
 func (ts *ProvisioningTestSuite) TestParseInstallYaml(c *C) {
 
-	_, err := parseInstallYaml(InstallYamlFile)
-	c.Check(err, Equals, ErrNoInstallYaml)
-
-	err = ioutil.WriteFile(InstallYamlFile, []byte(yamlData), 0750)
-	c.Check(err, IsNil)
-	_, err = parseInstallYaml(InstallYamlFile)
-	c.Check(err, IsNil)
-
-	err = ioutil.WriteFile(InstallYamlFile, []byte(yamlDataNoDevicePart), 0750)
-	c.Check(err, IsNil)
-	_, err = parseInstallYaml(InstallYamlFile)
-	c.Check(err, IsNil)
-
-	err = ioutil.WriteFile(InstallYamlFile, []byte(garbageData), 0750)
-	c.Check(err, IsNil)
-	_, err = parseInstallYaml(InstallYamlFile)
+	_, err := parseInstallYaml(ts.mockYamlFile)
+	c.Check(err, ErrorMatches, `failed to read provisioning data: open .*/install.yaml: no such file or directory`)
+
+	err = ioutil.WriteFile(ts.mockYamlFile, []byte(yamlData), 0750)
+	c.Check(err, IsNil)
+	_, err = parseInstallYaml(ts.mockYamlFile)
+	c.Check(err, IsNil)
+
+	err = ioutil.WriteFile(ts.mockYamlFile, []byte(yamlDataNoDevicePart), 0750)
+	c.Check(err, IsNil)
+	_, err = parseInstallYaml(ts.mockYamlFile)
+	c.Check(err, IsNil)
+
+	err = ioutil.WriteFile(ts.mockYamlFile, []byte(garbageData), 0750)
+	c.Check(err, IsNil)
+	_, err = parseInstallYaml(ts.mockYamlFile)
 	c.Check(err, Not(Equals), nil)
 }
 
@@ -163,3 +159,25 @@
 	_, err = parseInstallYamlData([]byte(garbageData))
 	c.Check(err, Not(Equals), nil)
 }
+
+func (ts *ProvisioningTestSuite) TestInDeveloperModeEmpty(c *C) {
+	c.Assert(InDeveloperMode(""), Equals, false)
+}
+
+func (ts *ProvisioningTestSuite) TestInDeveloperModeWithDevModeOn(c *C) {
+	err := ioutil.WriteFile(ts.mockYamlFile, []byte(`
+options:
+ developer-mode: true
+`), 0644)
+	c.Assert(err, IsNil)
+	c.Assert(InDeveloperMode(ts.mockBootDir), Equals, true)
+}
+
+func (ts *ProvisioningTestSuite) TestInDeveloperModeWithDevModeOff(c *C) {
+	err := ioutil.WriteFile(ts.mockYamlFile, []byte(`
+options:
+ developer-mode: false
+`), 0644)
+	c.Assert(err, IsNil)
+	c.Assert(InDeveloperMode(ts.mockBootDir), Equals, false)
+}

=== modified file 'snappy/install.go'
--- snappy/install.go	2015-06-10 13:38:10 +0000
+++ snappy/install.go	2015-06-22 23:53:50 +0000
@@ -21,13 +21,12 @@
 
 import (
 	"fmt"
-	"io/ioutil"
 	"os"
 	"sort"
-	"strings"
 
 	"launchpad.net/snappy/logger"
 	"launchpad.net/snappy/progress"
+	"launchpad.net/snappy/provisioning"
 )
 
 // InstallFlags can be used to pass additional flags to the install of a
@@ -45,28 +44,6 @@
 	AllowOEM
 )
 
-// check if the image is in developer mode
-// FIXME: this is a bit crude right now, but it seems like there is not more
-//        meta-data to check right now
-// TODO: add feature to ubuntu-device-flash to write better info file when
-//       the image is in developer mode
-func inDeveloperMode() bool {
-	f, err := os.Open(cloudMetaDataFile)
-	if err != nil {
-		return false
-	}
-	defer f.Close()
-	data, err := ioutil.ReadAll(f)
-	if err != nil {
-		return false
-	}
-	needle := "public-keys:\n"
-	if strings.Contains(string(data), needle) {
-		return true
-	}
-	return false
-}
-
 // Update the installed snappy packages, it returns the updated Parts
 // if updates where available and an error and nil if any of the updates
 // fail to apply.
@@ -115,7 +92,12 @@
 	if fi, err := os.Stat(name); err == nil && fi.Mode().IsRegular() {
 		// we allow unauthenticated package when in developer
 		// mode
-		if inDeveloperMode() {
+		//
+		// FIXME: this is terrible, we really need a single
+		//        bootloader dir like /boot or /boot/loader
+		//        instead of having to query the partition code
+		p := newPartition()
+		if provisioning.InDeveloperMode(p.BootloaderDir()) {
 			flags |= AllowUnauthenticated
 		}
 

=== modified file 'snappy/install_test.go'
--- snappy/install_test.go	2015-06-10 15:13:07 +0000
+++ snappy/install_test.go	2015-06-22 23:53:50 +0000
@@ -42,21 +42,6 @@
 	return w.Name()
 }
 
-func (s *SnapTestSuite) TestNotInDeveloperMode(c *C) {
-	cloudMetaDataFile = makeCloudInitMetaData(c, `instance-id: nocloud-static`)
-	defer os.Remove(cloudMetaDataFile)
-	c.Assert(inDeveloperMode(), Equals, false)
-}
-
-func (s *SnapTestSuite) TestInDeveloperMode(c *C) {
-	cloudMetaDataFile = makeCloudInitMetaData(c, `instance-id: nocloud-static
-public-keys:
-  - ssh-rsa AAAAB3NzAndSoOn
-`)
-	defer os.Remove(cloudMetaDataFile)
-	c.Assert(inDeveloperMode(), Equals, true)
-}
-
 func (s *SnapTestSuite) TestInstallInstall(c *C) {
 	snapFile := makeTestSnapPackage(c, "")
 	name, err := Install(snapFile, AllowUnauthenticated|DoInstallGC, &progress.NullProgress{})