Merge lp:~mvo/snappy/snappy-review-tools-reenable into lp:~snappy-dev/snappy/snappy-moved-to-github

[Michael Vogt]

Once the tools are updated from the latest upload in tools-proposed this MP can land as the click-review-tools are now working very well with snaps.

[John Lenton (chipaca)]

Yes please. Top approve if/when the tools are ready.

review: Approve

[Federico Gimenez (fgimenez)]

FAILED: Continuous integration, rev:573
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~mvo/snappy/snappy-review-tools-reenable/+merge/264301/+edit-commit-message

http://10.55.60.183:8080/job/snappy-rolling-ci/26/
Executed test runs:

Click here to trigger a rebuild:
http://10.55.60.183:8080/job/snappy-rolling-ci/26/rebuild

review: Needs Fixing (continuous-integration)

[Leo Arias (elopio)]

We have an integration test that builds a snap: http://bazaar.launchpad.net/~snappy-dev/snappy/snappy/view/head:/_integration-tests/tests/build_test.go#L49
The build is done in the snappy machine, and it is not expecting the warning we get when click-review is not installed. So, the test needs to be updated to match a more relaxed rexexp. And we need a new card to test more builds including the review, but that should probably wait until comfy is properly defined.

START: /tmp/snappy-tests-job/24707/src/launchpad.net/snappy/_integration-tests/tests/build_test.go:49: buildSuite.TestBuildBasicSnapOnSnappy
START: <autogenerated>:7: buildSuite.SetUpTest
****** Running buildSuite.TestBuildBasicSnapOnSnappy
[...]

snappy build _integration-tests/data/snaps/basic
Warning: could not review package (click-review not available)
Generated 'basic_1.0_all.snap' snap
/tmp/snappy-tests-job/24707/src/launchpad.net/snappy/_integration-tests/tests/build_test.go:55:
    ...open /tmp/snappy-tests-job/24707/src/launchpad.net/snappy/_integration-tests/tests/build_test.go: no such file or directory
... obtained string = "" +
... "Warning: could not review package (click-review not available)\n" +
... "Generated 'basic_1.0_all.snap' snap\n"
... expected string = "Generated 'basic_1.0_all.snap' snap\n"

review: Needs Fixing

[Michael Vogt (mvo)]

This branch was somehow forgotten. We should resurrect it :) What needs to happen for the test? Can we just install click-reviewers-tools from the tools PPA?

[Leo Arias (elopio)]

After merging with trunk, something like this makes the test pass.

http://paste.ubuntu.com/12705348/

We can't install the reviewer tools because we are doing the build on snappy itself. Well, we could install them, but not with apt-get.
Now that your classic dimension snappy is coming, we can move the build to that environment, and test more things. For now, this will do.

[lp:~mvo/snappy/snappy-review-tools-reenable]

574. By Michael Vogt on 2015-10-16

merged lp:snappy

575. By Michael Vogt on 2015-10-16

apply patch from Leo to fix integration tests

[Michael Vogt (mvo)]

Thanks Leo! I updated the branch and applied your patch. Hopefully its ready now.

[Leo Arias (elopio)]

:D

review: Approve

Unmerged revisions

575. By Michael Vogt on 2015-10-16

apply patch from Leo to fix integration tests

574. By Michael Vogt on 2015-10-16

merged lp:snappy

573. By Michael Vogt on 2015-07-09

re-enable the review tools

=== modified file '_integration-tests/testutils/build/snap.go'
--- _integration-tests/testutils/build/snap.go	2015-10-08 07:32:04 +0000
+++ _integration-tests/testutils/build/snap.go	2015-10-16 16:49:08 +0000
@@ -22,6 +22,7 @@
 import (
 	"fmt"
 	"path/filepath"
+	"regexp"
 
 	"gopkg.in/check.v1"
 	"launchpad.net/snappy/_integration-tests/testutils/cli"
@@ -48,8 +49,13 @@
 	snapName = snapName + snapFilenameSufix
 
 	path := filepath.Join(buildPath, snapName)
-	expected := fmt.Sprintf("Generated '%s' snap\n", path)
-	if buildOutput != expected {
+
+	expected := fmt.Sprintf("(?ms).*Generated '%s' snap\n$", path)
+	matched, err := regexp.MatchString(expected, buildOutput)
+	if err != nil {
+		return "", err
+	}
+	if !matched {
 		return "", fmt.Errorf("Error building snap, expected output %s, obtained %s",
 			expected, buildOutput)
 	}

=== modified file 'cmd/snappy/cmd_build.go'
--- cmd/snappy/cmd_build.go	2015-10-07 15:47:01 +0000
+++ cmd/snappy/cmd_build.go	2015-10-16 16:49:08 +0000
@@ -21,6 +21,8 @@
 
 import (
 	"fmt"
+	"os"
+	"os/exec"
 
 	"launchpad.net/snappy/i18n"
 	"launchpad.net/snappy/logger"
@@ -64,20 +66,16 @@
 		return err
 	}
 
-	/*
-		Disabling review tools run until the output reflects reality more closely
-
-		_, err = exec.LookPath(clickReview)
-		if err != nil {
-			fmt.Fprintf(os.Stderr, "Warning: could not review package (%s not available)\n", clickReview)
-		}
-
+	_, err = exec.LookPath(clickReview)
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "Warning: could not review package (%s not available)\n", clickReview)
+	} else {
 		cmd := exec.Command(clickReview, snapPackage)
 		cmd.Stdout = os.Stdout
 		cmd.Stderr = os.Stderr
 		// we ignore the error for now
 		_ = cmd.Run()
-	*/
+	}
 
 	// TRANSLATORS: the %s is a pkgname
 	fmt.Printf(i18n.G("Generated '%s' snap\n"), snapPackage)