Merge lp:~fgimenez/snappy/config-for-remote-testbeds into lp:~snappy-dev/snappy/snappy-moved-to-github

[Federico Gimenez]

Do not run tests that make assertions about release and channel in config when connecting to remote testbed

Do not run tests that make assertions about release and channel in config when connecting to remote testbed

[Leo Arias (elopio)]

One comment for discussion.

review: Needs Information

[lp:~fgimenez/snappy/config-for-remote-testbeds]

621. By Federico Gimenez on 2015-07-30

Skipping info test when executing in remote testbed

[Federico Gimenez (fgimenez)]

I agree, the problem here are the default values of release and channel, if not specified they are not empty strings.

I've opted for your second suggestion, skipping the test when executing in remote testbeds, but using the given testbedIP to check if the testbed is remote, let me know what do you think.

Thanks!

[lp:~fgimenez/snappy/config-for-remote-testbeds]

622. By Federico Gimenez on 2015-07-30

merged prerequisite

623. By Federico Gimenez on 2015-07-30

merge prerequisite

[Leo Arias (elopio)]

> I agree, the problem here are the default values of release and channel, if
> not specified they are not empty strings.

Right, I understand now.
This is ok, so +1. A couple of alternatives that might be clearer or not would be:
- Instead of passing the test bed ip in the config, pass a boolean value that's "remote", set to true when the --ip flag is used.
- When the --ip flag is used, save "" as the release and channel in the config, instead of the default values.

I really don't know which is better between your current approach and those other two options, so I leave the top-approval to you. If you prefer it as it is, please go ahead and merge.

review: Approve

[lp:~fgimenez/snappy/config-for-remote-testbeds]

624. By Federico Gimenez on 2015-07-31

merged trunk

625. By Federico Gimenez on 2015-07-31

RemoteTestbed field for config

626. By Federico Gimenez on 2015-07-31

merged trunk

[Federico Gimenez (fgimenez)]

Ok, I've gone for the first option, having a RemoteTestbed boolean field in the config. Tested in rolling and 15.04, top-approving

Thanks!

=== modified file '_integration-tests/main.go'
--- _integration-tests/main.go	2015-07-31 07:32:21 +0000
+++ _integration-tests/main.go	2015-07-31 08:00:41 +0000
@@ -79,17 +79,19 @@
 	testutils.PrepareTargetDir(dataOutputDir)
 	defer os.RemoveAll(dataOutputDir)
 
+	remoteTestbed := *testbedIP != ""
+
 	// TODO: pass the config as arguments to the test binaries.
 	// --elopio - 2015-07-15
 	cfg := config.NewConfig(
 		configFileName, *imgRelease, *imgChannel, *targetRelease, *targetChannel,
-		*update, *rollback)
+		remoteTestbed, *update, *rollback)
 	cfg.Write()
 
 	rootPath := testutils.RootPath()
 
 	test := autopkgtest.NewAutopkgtest(rootPath, baseDir, *testFilter, build.IntegrationTestName)
-	if *testbedIP == "" {
+	if !remoteTestbed {
 		img := image.NewImage(*imgRelease, *imgChannel, *imgRevision, baseDir)
 
 		if imagePath, err := img.UdfCreate(); err == nil {

=== modified file '_integration-tests/tests/info_test.go'
--- _integration-tests/tests/info_test.go	2015-07-28 04:03:52 +0000
+++ _integration-tests/tests/info_test.go	2015-07-31 08:00:41 +0000
@@ -34,6 +34,14 @@
 }
 
 func (s *infoSuite) TestInfoMustPrintReleaseAndChannel(c *check.C) {
+	// skip test when having a remote testbed (we can't know which the
+	// release and channels are)
+	if Cfg.RemoteTestbed {
+		c.Skip(fmt.Sprintf(
+			"Skipping %s while testing in remote testbed",
+			c.TestName()))
+	}
+
 	infoOutput := ExecCommand(c, "snappy", "info")
 
 	expected := "(?ms)" +

=== modified file '_integration-tests/testutils/config/config.go'
--- _integration-tests/testutils/config/config.go	2015-07-29 11:06:52 +0000
+++ _integration-tests/testutils/config/config.go	2015-07-31 08:00:41 +0000
@@ -33,16 +33,17 @@
 	Channel       string
 	TargetRelease string
 	TargetChannel string
+	RemoteTestbed bool
 	Update        bool
 	Rollback      bool
 }
 
 // NewConfig is the Config constructor
-func NewConfig(fileName, release, channel, targetRelease, targetChannel string, update, rollback bool) *Config {
+func NewConfig(fileName, release, channel, targetRelease, targetChannel string, remoteTestbed, update, rollback bool) *Config {
 	return &Config{
 		FileName: fileName, Release: release, Channel: channel,
 		TargetRelease: targetRelease, TargetChannel: targetChannel,
-		Update: update, Rollback: rollback,
+		RemoteTestbed: remoteTestbed, Update: update, Rollback: rollback,
 	}
 }
 

=== modified file '_integration-tests/testutils/config/config_test.go'
--- _integration-tests/testutils/config/config_test.go	2015-07-29 11:06:52 +0000
+++ _integration-tests/testutils/config/config_test.go	2015-07-31 08:00:41 +0000
@@ -47,7 +47,7 @@
 	return NewConfig(
 		fileName,
 		"testrelease", "testchannel", "testtargetrelease", "testtargetchannel",
-		true, true)
+		true, true, true)
 }
 func testConfigContents(fileName string) string {
 	return `{` +
@@ -56,6 +56,7 @@
 		`"Channel":"testchannel",` +
 		`"TargetRelease":"testtargetrelease",` +
 		`"TargetChannel":"testtargetchannel",` +
+		`"RemoteTestbed":true,` +
 		`"Update":true,` +
 		`"Rollback":true` +
 		`}`
@@ -91,3 +92,30 @@
 	c.Assert(err, check.IsNil, check.Commentf("Error reading config: %v", err))
 	c.Assert(cfg, check.DeepEquals, testConfigStruct(configFileName))
 }
+
+func (s *ConfigSuite) TestReadConfigLocalTestBed(c *check.C) {
+	configFileName := testConfigFileName(c)
+
+	configContents := `{` +
+		fmt.Sprintf(`"FileName":"%s",`, configFileName) +
+		`"Release":"testrelease",` +
+		`"Channel":"testchannel",` +
+		`"TargetRelease":"testtargetrelease",` +
+		`"TargetChannel":"testtargetchannel",` +
+		`"RemoteTestbed":false,` +
+		`"Update":true,` +
+		`"Rollback":true` +
+		`}`
+
+	ioutil.WriteFile(configFileName, []byte(configContents), 0644)
+
+	cfg, err := ReadConfig(configFileName)
+
+	testConfigStruct := NewConfig(
+		configFileName,
+		"testrelease", "testchannel", "testtargetrelease", "testtargetchannel",
+		false, true, true)
+
+	c.Assert(err, check.IsNil, check.Commentf("Error reading config: %v", err))
+	c.Assert(cfg, check.DeepEquals, testConfigStruct)
+}