Merge lp:~elopio/snapcraft/integration_coverage into lp:~snappy-dev/snapcraft/core

[Leo Arias]

Added coverage report to the integration tests.

[lp:~elopio/snapcraft/integration_coverage]

95. By Leo Arias on 2015-07-22

Removed extra newline.

[Leo Arias (elopio)]

I tried way to many things to get this working. I'm not totally happy with the approach, but it works. If you can think of a better way to do this, please leave a comment.

[Michael Vogt (mvo)]

Nice approach, I like it!

review: Approve

[Michael Terry (mterry)]

I'm fine with the approach of a second, coverage-enabled binary. But don't we want to combine the unit tests and integration tests for the coverage report?

review: Needs Information

[Michael Terry (mterry)]

(that is, have one report, not two)

[Leo Arias (elopio)]

> I'm fine with the approach of a second, coverage-enabled binary. But don't we
> want to combine the unit tests and integration tests for the coverage report?

I've pushed a small change that combines the two reports.

I don't like this, but for now seems ok to me.

What I would prefer is to do test-driven development. So the unit test coverage should be close to 100%, and the unit coverage report will be just a guide of how well we are doing TDD.
On top of that, I would use the functional tests to cover the happy paths of the user-facing features. Plus some regressions tests or tests for important error conditions. Then the integration coverage report won't be close to 100%, and the simple table is not useful. What's useful is the HTML report that we can use to discover important user-facing features not covered.

There is no correct approach here, so we can discuss and feel free to disagree. If we decide to go the full TDD way, we can split the reports later and display the HTML nicely in the jenkins server.

thanks for the reviews!

[lp:~elopio/snapcraft/integration_coverage]

96. By Leo Arias on 2015-07-22

Combine the two coverage reports.

[Michael Terry (mterry)]

Well, I guess I considered integration tests a part of TDD too. But maybe that's not the Correct way.

You're the testing expert. :) If you would rather see us drive unit tests to 100% separately from integration tests, that's fine and we can split them.

[Leo Arias (elopio)]

Here's your typical "expert" comment: there's no correct way :)
Lets play with this for now, and try to increase the overall coverage.

[Michael Vogt (mvo)]

I was assuming we would keep them separate as they are a different kind of testing. But its fine and easy enough to change later so no issue from my side.

[Leo Arias (elopio)]

approved too soon? sorry about that.

=== added file 'bin/snapcraft-coverage'
--- bin/snapcraft-coverage	1970-01-01 00:00:00 +0000
+++ bin/snapcraft-coverage	2015-07-22 14:22:27 +0000
@@ -0,0 +1,44 @@
+#!/usr/bin/python3
+# -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*-
+#
+# 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/>.
+
+import os
+import sys
+
+import coverage
+
+# make running from bzr snapshot easy
+topdir = os.path.abspath(os.path.join(__file__, "..", ".."))
+if os.path.exists(os.path.join(topdir, 'setup.py')):
+    sys.path = [topdir] + sys.path
+
+# now import
+import snapcraft.main
+import snapcraft.dirs
+
+
+if __name__ == "__main__":
+    cov = coverage.coverage(
+        data_file=os.path.join(topdir, '.coverage'), data_suffix=True,
+        branch=True, source=['snapcraft'])
+    cov.start()
+    try:
+        snapcraft.dirs.setup_dirs()
+        snapcraft.main.main()
+    except:
+        cov.stop()
+        cov.save()
+        raise

=== modified file 'integration-tests/units/jobs.pxu'
--- integration-tests/units/jobs.pxu	2015-07-10 19:21:16 +0000
+++ integration-tests/units/jobs.pxu	2015-07-22 14:22:27 +0000
@@ -3,7 +3,7 @@
 estimated_duration: 0.1
 command:
     set -x
-    OUTPUT=$(snapcraft pull)
+    OUTPUT=$(${SNAPCRAFT} pull)
     test $? = 1 || exit 1
     echo $OUTPUT | grep "Could not find snapcraft\.yaml\."
 
@@ -13,10 +13,10 @@
 command:
     set -ex
     cp -rT $PLAINBOX_PROVIDER_DATA/local-source .
-    snapcraft build
+    ${SNAPCRAFT} build
     test -e stamp-all
     test "$(readlink parts/make-project/build)" = "$(pwd)"
-    snapcraft stage
+    ${SNAPCRAFT} stage
     test -e stamp-install
 
 id: snapcraft/normal/local-plugin
@@ -25,7 +25,7 @@
 command:
     set -ex
     cp -rT $PLAINBOX_PROVIDER_DATA/local-plugin .
-    snapcraft snap
+    ${SNAPCRAFT} snap
     test -e snap/build-stamp
 
 id: snapcraft/normal/simple-make
@@ -34,7 +34,7 @@
 command:
     set -ex
     cp -rT $PLAINBOX_PROVIDER_DATA/simple-make .
-    snapcraft snap
+    ${SNAPCRAFT} snap
     test "$(./snap/bin/test)" = "Hello world"
 
 id: snapcraft/normal/simple-cmake
@@ -43,7 +43,7 @@
 command:
     set -ex
     cp -rT $PLAINBOX_PROVIDER_DATA/simple-cmake .
-    snapcraft snap
+    ${SNAPCRAFT} snap
     test "$(./snap/bin/simple-cmake)" = "It's a CMake world"
 
 id: snapcraft/normal/conflicts
@@ -52,7 +52,7 @@
 command:
     set -x
     cp -rT $PLAINBOX_PROVIDER_DATA/conflicts .
-    OUTPUT=$(snapcraft stage)
+    OUTPUT=$(${SNAPCRAFT} stage)
     test $? = 1 || exit 1
     echo $OUTPUT | grep "Error: parts p1 and p2 have the following files in common: bin/test" # squished output by bash
 
@@ -62,10 +62,10 @@
 command:
     set -ex
     cp -rT $PLAINBOX_PROVIDER_DATA/dependencies .
-    snapcraft snap
+    ${SNAPCRAFT} snap
     test -x stage/bin/p3
     test -x snap/bin/p3
-    test "$(snapcraft shell p3)" = 'p1
+    test "$(${SNAPCRAFT} shell p3)" = 'p1
     p1
     p2'
 
@@ -76,7 +76,7 @@
     set -x
     cp -rT $PLAINBOX_PROVIDER_DATA/dependencies .
     sed -i "s/p1:/p1:\n    after: [p3]/" snapcraft.yaml
-    OUTPUT=$(snapcraft pull)
+    OUTPUT=$(${SNAPCRAFT} pull)
     test $? = 1 || exit 1
     echo $OUTPUT | grep -i "circular dependency"
 
@@ -87,5 +87,5 @@
     set -x
     cp -rT $PLAINBOX_PROVIDER_DATA/dependencies .
     sed -i '/after/d' snapcraft.yaml
-    snapcraft snap
+    ${SNAPCRAFT} snap
     test $? = 1 || exit 1

=== modified file 'runtests.sh'
--- runtests.sh	2015-07-21 02:03:13 +0000
+++ runtests.sh	2015-07-22 14:22:27 +0000
@@ -28,11 +28,10 @@
 pyflakes3 $SRC_PATHS
 
 if which python3-coverage >/dev/null 2>&1; then
-    python3-coverage erase
-    python3-coverage run --branch --source snapcraft -m unittest    
-    python3-coverage report
+    python3-coverage run --branch --source snapcraft -m unittest
+    mv .coverage .coverage.unit
 else
-    python3 -m unittest    
+    python3 -m unittest
 fi
 
 if [ -z "$SNAPCRAFT_TESTS_SKIP_PLAINBOX" ]; then
@@ -49,6 +48,14 @@
         exit 1
     fi
 
+    if which python3-coverage >/dev/null 2>&1; then
+        python3-coverage erase
+        export PROJECT_PATH=$(pwd)
+        export SNAPCRAFT=snapcraft-coverage
+    else
+        export SNAPCRAFT=snapcraft
+    fi
+
     # Go to the plainbox provider of snapcraft tests
     cd integration-tests/
     # Create a temporary directory so that we can run 'manage.py develop' and
@@ -77,6 +84,13 @@
 raise SystemExit(failed)
 __PYTHON__
 )
+
+fi
+
+if which python3-coverage >/dev/null 2>&1; then
+    python3-coverage combine
+    python3-coverage report
+    python3-coverage erase
 fi
 
 echo -e "\e[1;32mEverything passed\e[0m"