Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
integration-tests: get name of OS snap from bootloader #343
Conversation
elopio
referenced this pull request
Jan 20, 2016
Merged
integration-tests: fix the bootloader path #351
fgimenez
reviewed
Jan 21, 2016
| - "^Reboot to use ubuntu-core version .*\\.\n" | ||
| + expected := fmt.Sprintf("(?ms)"+ | ||
| + ".*"+ | ||
| + "^Reboot to use %s version .*\\.\n", common.OSSnapName(c)) |
fgimenez
Jan 21, 2016
Contributor
I would leave the first two lines outside the fmt.Sprintf call, but it's ok as it is
fgimenez
reviewed
Jan 21, 2016
| + c.Assert(err, check.IsNil, check.Commentf("Error getting the name of the OS snap: %s", err)) | ||
| + return strings.Split(snappyOS, ".")[0] | ||
| +} | ||
| + |
fgimenez
Jan 21, 2016
Contributor
I prefer to move things from common to the proper package instead of adding more code to it, what do you think? Would the testutils/partition package be a good place for this function? Or perhaps a new one, including GetCurrentUbuntuCoreVersion?
elopio
Jan 22, 2016
Member
partition is clearly not a good place for this. But the right place depends on how https://bugs.launchpad.net/snappy/+bug/1532245 is fixed.
For now I'll put it in partition, which we also have to rename. And when the bug is fixed, we'll have to update this and it will be easier to find a name for that new package.
|
Looks good, the only real concern is about adding code to the common package, thanks! :) |
elopio
added some commits
Jan 22, 2016
|
@fgimenez please, take another look. |
fgimenez
reviewed
Jan 22, 2016
| @@ -134,6 +134,17 @@ func (s *bootloaderTestSuite) TestModeReturnsSnappyModeFromConf(c *check.C) { | ||
| c.Assert(mode, check.Equals, "test_mode", check.Commentf("Wrong mode")) | ||
| } | ||
| +func (s *bootloaderTestSuite) TestOSSnapNameReturnsSnapFromConf(c *check.C) { |
fgimenez
reviewed
Jan 22, 2016
| @@ -74,6 +75,19 @@ func Mode() (mode string, err error) { | ||
| return confValue("snappy_mode") | ||
| } | ||
| +// OSSnapName returns the name of the OS snap. | ||
| +func OSSnapName(c *check.C) string { | ||
| + snappyOS, err := snappyOS() |
|
LGTM thanks Leo |
|
I think we need the new changelog format in the pull-request description now :) Then this can land |
elopio commentedJan 19, 2016
First branch for getting the integration tests working in raspberry pi2.
Currently the os snap is called ubuntu-core in amd64-generic and ubuntu-core-armhf in rpi2. According to Matias, by the end of the next week the store will support the same name for multiple architectures, so we might go back to hardcode the value. Instead of that, I would prefer to get a nice way to query for the name of the os snap, which has a bug reported in https://bugs.launchpad.net/snappy/+bug/1534029 . But we'll see what happens first...