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

[Leo Arias]

Fixed the arm compilation in integration tests.

[Federico Gimenez (fgimenez)]

Looks very good :)

The tests are not passing for me, it seems that maybe we need [1] to be applied before this.

Also, with that in place I get an error about the set of CGO_ENABLED to 1 being executed two times, maybe 1 is the default value for it, we could set all the environment variables to known values before the assertions.

Thanks!

[1] https://code.launchpad.net/~fgimenez/snappy/setenv-closure-redefinition/+merge/269717

review: Needs Fixing

[Federico Gimenez (fgimenez)]

There was a problem with the fake environment variables, the prerequisite should be [1]

Thanks

[1] https://code.launchpad.net/~fgimenez/snappy/fix-fake-env/+merge/269741

[Federico Gimenez (fgimenez)]

For setting the environment variables with the changes in the prerequisite now we can access the s.environ map directly before the Assets call:

s.environ["CC"] = os.Getenv("CC")

Also a minor inline comment, thanks!

[lp:~elopio/snappy/goarm_tests]

649. By Leo Arias on 2015-09-01

Set the variables to a know value different to the one we need.

[Leo Arias (elopio)]

Federico, I pushed a new revision. I prefer to keep the scenarios out of the test, but if you feel strongly about it, I can just put them in the for as you suggested. No big deal for me.

[Federico Gimenez (fgimenez)]

No problem, that's fine.

I'm getting one error on the tests, the one related to the setting of CGO_ENABLED being called two times, one before the compilation and other after it. It seems that the value that we are setting here is already the default.

Executed in wily, go1.5, don't you get this error?

[Leo Arias (elopio)]

I don't get that error.
But I think that with s.environ[t.envVar] = "original" + t.envVar, GGO_ENABLED will return to a value that's not 1, so it shouldn't be called twice. Can it be called somewhere else in between?

[Federico Gimenez (fgimenez)]

Ok, it's working now, thanks! :)

review: Approve

=== modified file '_integration-tests/testutils/build/build.go'
--- _integration-tests/testutils/build/build.go	2015-08-10 10:01:18 +0000
+++ _integration-tests/testutils/build/build.go	2015-09-01 16:11:25 +0000
@@ -80,8 +80,15 @@
 		defer osSetenv("GOARCH", osGetenv("GOARCH"))
 		osSetenv("GOARCH", arch)
 		if arch == "arm" {
-			defer osSetenv("GOARM", osGetenv("GOARM"))
-			osSetenv("GOARM", defaultGoArm)
+			envs := map[string]string{
+				"GOARM":       defaultGoArm,
+				"CGO_ENABLED": "1",
+				"CC":          "arm-linux-gnueabihf-gcc",
+			}
+			for env, value := range envs {
+				defer osSetenv(env, osGetenv(env))
+				osSetenv(env, value)
+			}
 		}
 	}
 	execCommand(strings.Fields(cmd)...)

=== modified file '_integration-tests/testutils/build/build_test.go'
--- _integration-tests/testutils/build/build_test.go	2015-09-01 16:11:25 +0000
+++ _integration-tests/testutils/build/build_test.go	2015-09-01 16:11:25 +0000
@@ -161,19 +161,35 @@
 			"GOARCH "+os.Getenv("GOARCH"), setenvGOARCHFinalCall))
 }
 
+var armEnvironmentTests = []struct {
+	envVar string
+	value  string
+}{
+	{"GOARM", defaultGoArm},
+	{"CGO_ENABLED", "1"},
+	{"CC", "arm-linux-gnueabihf-gcc"},
+}
+
 func (s *BuildSuite) TestAssetsSetsEnvironmentForArm(c *check.C) {
 	arch := "arm"
+	for _, t := range armEnvironmentTests {
+		s.environ[t.envVar] = "original" + t.envVar
+	}
 	Assets(s.useSnappyFromBranch, arch)
 
-	setenvGOARMFirstCall := s.osSetenvCalls["GOARM "+defaultGoArm]
-	setenvGOARMFinalCall := s.osSetenvCalls["GOARM "+os.Getenv("GOARM")]
+	for _, t := range armEnvironmentTests {
+		firstCall := fmt.Sprintf("%s %s", t.envVar, t.value)
+		setenvFirstCall := s.osSetenvCalls[firstCall]
+		finalCall := fmt.Sprintf("%s %s", t.envVar, "original"+t.envVar)
+		setenvFinalCall := s.osSetenvCalls[finalCall]
 
-	c.Assert(setenvGOARMFirstCall, check.Equals, 1,
-		check.Commentf("Expected 1 call to os.Setenv with %s, got %d",
-			"GOARM "+defaultGoArm, setenvGOARMFirstCall))
-	c.Assert(setenvGOARMFinalCall, check.Equals, 1,
-		check.Commentf("Expected 1 call to os.Setenv with %s, got %d",
-			"GOARM "+os.Getenv("GOARCH"), setenvGOARMFinalCall))
+		c.Assert(setenvFirstCall, check.Equals, 1,
+			check.Commentf("Expected 1 call to os.Setenv with %s, got %d",
+				firstCall, setenvFirstCall))
+		c.Assert(setenvFinalCall, check.Equals, 1,
+			check.Commentf("Expected 1 call to os.Setenv with %s, got %d",
+				finalCall, setenvFinalCall))
+	}
 }
 
 func (s *BuildSuite) TestAssetsDoesNotSetEnvironmentForEmptyArch(c *check.C) {