Merge lp:~elopio/snappy/normalize_test into lp:~snappy-dev/snappy/snappy-moved-to-github

[Leo Arias]

Added a check for the bootdir contents.

[Federico Gimenez (fgimenez)]

Very nice Leo, I've added unit tests for the partition package here [1]

Thanks!

[1] https://code.launchpad.net/~fgimenez/snappy/partition-tests/+merge/270379

review: Approve

[Federico Gimenez (fgimenez)]

I'm getting this in the updateSuite.TestUpdateToSameReleaseAndChannel on bbb, works fine on kvm:

/home/fgimenez/workspace/snappy/partition_tests/_integration-tests/tests/update_test.go:67:
    ...open /home/fgimenez/workspace/snappy/partition_tests/_integration-tests/tests/update_test.go: no such file or directory
/home/fgimenez/workspace/snappy/partition_tests/_integration-tests/tests/update_test.go:53:
    ...open /home/fgimenez/workspace/snappy/partition_tests/_integration-tests/tests/update_test.go: no such file or directory
... obtained []string = []string{"dtbs", "hardware.yaml", "initrd.img", "vmlinuz"}
... expected []string = []string{"hardware.yaml", "initrd.img", "vmlinuz"}
... Wrong files in the other partition boot dir

review: Needs Fixing

[Leo Arias (elopio)]

Thanks for catching that federico.

ogra and sergio confirmed that the dtbs is required on our uboot devices, so I added that condition.
I know you said you didn't like the if, but the other alternatives are checking that there doesn't exist a file with a name like vmlinuz.* or initrd.img.*, and I didn't like how that code looked.

Let me know what you think, and if you are against the if I'll get the other check.

[Federico Gimenez (fgimenez)]

No problem with the ifs! Thanks :)

review: Approve

[Snappy Tarmac (snappydevtarmac)]

The attempt to merge lp:~elopio/snappy/normalize_test into lp:snappy failed. Below is the output from the failed tests.

Checking docs
Checking formatting
Formatting wrong in following files
_integration-tests/tests/update_test.go

# we always run in a fresh dir in tarmac
export GOPATH=$(mktemp -d)
trap 'rm -rf "$GOPATH"' EXIT

# this is a hack, but not sure tarmac is golang friendly
mkdir -p $GOPATH/src/launchpad.net/snappy
cp -a . $GOPATH/src/launchpad.net/snappy/
cd $GOPATH/src/launchpad.net/snappy

./run-checks

if which goctest >/dev/null; then
    goctest="goctest"
else
    goctest="go test"
fi

echo Checking docs
./mdlint.py docs/*.md

echo Checking formatting
fmt=$(gofmt -l .)

if [ -n "$fmt" ]; then
    echo "Formatting wrong in following files"
    echo "$fmt"
    exit 1
fi

[lp:~elopio/snappy/normalize_test]

641. By Leo Arias on 2015-09-10

Fixed the format.

=== modified file '_integration-tests/tests/failover_zero_size_file_test.go'
--- _integration-tests/tests/failover_zero_size_file_test.go	2015-07-31 12:06:24 +0000
+++ _integration-tests/tests/failover_zero_size_file_test.go	2015-09-10 15:55:38 +0000
@@ -20,15 +20,15 @@
 package tests
 
 import (
-	"bufio"
 	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
 
 	. "launchpad.net/snappy/_integration-tests/testutils/common"
+	"launchpad.net/snappy/_integration-tests/testutils/partition"
 
-	check "gopkg.in/check.v1"
+	"gopkg.in/check.v1"
 )
 
 const (
@@ -38,11 +38,6 @@
 	initrdFilename             = "initrd"
 	systemdFilename            = "systemd"
 	destFilenamePrefix         = "snappy-selftest-"
-	bootBase                   = "/boot"
-	ubootDir                   = bootBase + "/uboot"
-	grubDir                    = bootBase + "/grub"
-	ubootConfigFile            = ubootDir + "/snappy-system.txt"
-	grubConfigFile             = grubDir + "/grubenv"
 )
 
 type zeroSizeKernel struct{}
@@ -61,8 +56,10 @@
 	if classicKernelFiles(c) {
 		commonSet(c, BaseAltPartitionPath, origBootFilenamePattern, initrdFilename)
 	} else {
-		boot := bootSystem(c)
-		dir := bootDirectory(boot)
+		boot, err := partition.BootSystem()
+		c.Assert(err, check.IsNil, check.Commentf("Error getting the boot system: %s", err))
+		dir := partition.BootDir(boot)
+
 		bootFileNamePattern := newKernelFilenamePattern(c, boot, true)
 		commonSet(c, dir, bootFileNamePattern, initrdFilename)
 	}
@@ -72,8 +69,10 @@
 	if classicKernelFiles(c) {
 		commonUnset(c, BaseAltPartitionPath, origBootFilenamePattern, initrdFilename)
 	} else {
-		boot := bootSystem(c)
-		dir := bootDirectory(boot)
+		boot, err := partition.BootSystem()
+		c.Assert(err, check.IsNil, check.Commentf("Error getting the boot system: %s", err))
+		dir := partition.BootDir(boot)
+
 		bootFileNamePattern := newKernelFilenamePattern(c, boot, false)
 		commonUnset(c, dir, bootFileNamePattern, initrdFilename)
 	}
@@ -154,68 +153,6 @@
 	return len(matches) == 1
 }
 
-func bootSystem(c *check.C) string {
-	matches, err := filepath.Glob(bootBase + "/grub")
-
-	c.Assert(err, check.IsNil, check.Commentf("Error: %v", err))
-
-	if len(matches) == 1 {
-		return "grub"
-	}
-	return "uboot"
-}
-
-func bootDirectory(bootSystem string) string {
-	if bootSystem == "grub" {
-		return grubDir
-	}
-	return ubootDir
-}
-
-func bootConfigFile(bootSystem string) string {
-	if bootSystem == "grub" {
-		return grubConfigFile
-	}
-	return ubootConfigFile
-}
-
-func currentPartition(c *check.C, bootSystem string) (partition string) {
-	bootConfigFile := bootConfigFile(bootSystem)
-	file, err := os.Open(bootConfigFile)
-
-	c.Assert(err, check.IsNil,
-		check.Commentf("Error reading boot config file %s", bootConfigFile))
-
-	defer file.Close()
-
-	reader := bufio.NewReader(file)
-	scanner := bufio.NewScanner(reader)
-
-	scanner.Split(bufio.ScanLines)
-
-	for scanner.Scan() {
-		if strings.HasPrefix(scanner.Text(), "snappy_ab") {
-			fields := strings.Split(scanner.Text(), "=")
-			if len(fields) > 1 {
-				if bootSystem == "grub" {
-					partition = fields[1]
-				} else {
-					partition = otherPart(fields[1])
-				}
-			}
-			return
-		}
-	}
-	return
-}
-
-func otherPart(current string) string {
-	if current == "a" {
-		return "b"
-	}
-	return "a"
-}
-
 // newKernelFilenamePattern returns the filename pattern to modify files
 // in the partition declared in the boot config file.
 //
@@ -226,11 +163,12 @@
 // we want to change the files in the other partition
 func newKernelFilenamePattern(c *check.C, bootSystem string, afterUpdate bool) string {
 	var actualPartition string
-	partition := currentPartition(c, bootSystem)
+	part, err := partition.CurrentPartition()
+	c.Assert(err, check.IsNil, check.Commentf("Error getting the current partition: %s", err))
 	if afterUpdate {
-		actualPartition = partition
+		actualPartition = part
 	} else {
-		actualPartition = otherPart(partition)
+		actualPartition = partition.OtherPartition(part)
 	}
 	return filepath.Join(actualPartition, "%s%s*")
 }
@@ -247,7 +185,9 @@
 func (s *failoverSuite) TestZeroSizeInitrd(c *check.C) {
 	// Skip if on uboot due to https://bugs.launchpad.net/snappy/+bug/1480248
 	// (fgimenez 20150731)
-	if bootSystem(c) == "uboot" {
+	boot, err := partition.BootSystem()
+	c.Assert(err, check.IsNil, check.Commentf("Error getting the boot system: %s", err))
+	if boot == "uboot" {
 		c.Skip("Failover for empty initrd not working in uboot")
 	}
 	commonFailoverTest(c, zeroSizeInitrd{})
 
=== modified file '_integration-tests/tests/update_test.go'
--- _integration-tests/tests/update_test.go	2015-07-28 04:03:52 +0000
+++ _integration-tests/tests/update_test.go	2015-09-10 15:55:38 +0000
@@ -20,9 +20,13 @@
 package tests
 
 import (
+	"io/ioutil"
+	"path"
+
 	. "launchpad.net/snappy/_integration-tests/testutils/common"
+	"launchpad.net/snappy/_integration-tests/testutils/partition"
 
-	check "gopkg.in/check.v1"
+	"gopkg.in/check.v1"
 )
 
 var _ = check.Suite(&updateSuite{})
@@ -31,6 +35,28 @@
 	SnappySuite
 }
 
+func (s *updateSuite) assertBootDirContents(c *check.C) {
+	system, err := partition.BootSystem()
+	c.Assert(err, check.IsNil, check.Commentf("Error getting the boot system: %s", err))
+	current, err := partition.CurrentPartition()
+	c.Assert(err, check.IsNil, check.Commentf("Error getting the current partition: %s", err))
+	files, err := ioutil.ReadDir(
+		path.Join(partition.BootDir(system), partition.OtherPartition(current)))
+	c.Assert(err, check.IsNil, check.Commentf("Error reading the other partition boot dir: %s", err))
+
+	expectedFileNames := []string{"hardware.yaml", "initrd.img", "vmlinuz"}
+	if system == "uboot" {
+		expectedFileNames = append([]string{"dtbs"}, expectedFileNames...)
+	}
+
+	fileNames := []string{}
+	for _, f := range files {
+		fileNames = append(fileNames, f.Name())
+	}
+	c.Assert(fileNames, check.DeepEquals, expectedFileNames,
+		check.Commentf("Wrong files in the other partition boot dir"))
+}
+
 // Test that the update to the same release and channel must install a newer
 // version. If there is no update available, the channel version will be
 // modified to fake an update. If there is a version available, the image will
@@ -42,6 +68,7 @@
 			".*" +
 			"^Reboot to use .*ubuntu-core.\n"
 		c.Assert(updateOutput, check.Matches, expected)
+		s.assertBootDirContents(c)
 		Reboot(c)
 	} else if AfterReboot(c) {
 		RemoveRebootMark(c)
 
=== added directory '_integration-tests/testutils/partition'
=== added file '_integration-tests/testutils/partition/bootloader.go'
--- _integration-tests/testutils/partition/bootloader.go	1970-01-01 00:00:00 +0000
+++ _integration-tests/testutils/partition/bootloader.go	2015-09-10 15:55:38 +0000
@@ -0,0 +1,113 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2015 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+package partition
+
+import (
+	"bufio"
+	"os"
+	"path/filepath"
+	"strings"
+)
+
+const (
+	bootBase        = "/boot"
+	ubootDir        = bootBase + "/uboot"
+	grubDir         = bootBase + "/grub"
+	ubootConfigFile = ubootDir + "/snappy-system.txt"
+	grubConfigFile  = grubDir + "/grubenv"
+)
+
+// BootSystem returns the name of the boot system, grub or uboot.
+func BootSystem() (string, error) {
+	matches, err := filepath.Glob(bootBase + "/grub")
+	if err != nil {
+		return "", err
+	}
+	if len(matches) == 1 {
+		return "grub", nil
+	}
+	return "uboot", nil
+}
+
+// BootDir returns the directory used by the boot system.
+func BootDir(bootSystem string) string {
+	if bootSystem == "grub" {
+		return grubDir
+	}
+	return ubootDir
+}
+
+// CurrentPartition returns the current partition, a or b.
+func CurrentPartition() (partition string, err error) {
+	bootConfigFile, err := bootConf()
+	if err != nil {
+		return
+	}
+	file, err := os.Open(bootConfigFile)
+	if err != nil {
+		return
+	}
+
+	defer file.Close()
+
+	reader := bufio.NewReader(file)
+	scanner := bufio.NewScanner(reader)
+
+	scanner.Split(bufio.ScanLines)
+
+	for scanner.Scan() {
+		if strings.HasPrefix(scanner.Text(), "snappy_ab") {
+			fields := strings.Split(scanner.Text(), "=")
+			if len(fields) > 1 {
+				var system string
+				system, err = BootSystem()
+				if err != nil {
+					return
+				}
+				if system == "grub" {
+					partition = fields[1]
+				} else {
+					partition = OtherPartition(fields[1])
+				}
+			}
+			return
+		}
+	}
+	return
+}
+
+func bootConf() (string, error) {
+	bootSystem, err := BootSystem()
+	if err != nil {
+		return "", err
+	}
+	if bootSystem == "grub" {
+		return grubConfigFile, nil
+	}
+	return ubootConfigFile, nil
+}
+
+// OtherPartition returns the backup partition, a or b.
+func OtherPartition(current string) string {
+	if current == "a" {
+		return "b"
+	}
+	return "a"
+}