Merge lp:~canonical-platform-qa/ubuntu-system-tests/temp_config_files into lp:ubuntu-system-tests

[Sergio Cazzolato]

Fix to make possible to use temp config file for regular config data

[Sergio Cazzolato (sergio-j-cazzolato)]

Still missing to fix the tests

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:154
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sanity-tests-ci/689/
Executed test runs:
    FAILURE: http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sanity-tests-wily-amd64-ci/417/console
    FAILURE: http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sanity-tests-wily-armhf-ci/418/console
    FAILURE: http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sanity-tests-wily-i386-ci/417/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sanity-tests-ci/689/rebuild

review: Needs Fixing (continuous-integration)

[lp:~canonical-platform-qa/ubuntu-system-tests/temp_config_files]

155. By Sergio Cazzolato on 2015-08-23

Fixing unit tests

156. By Sergio Cazzolato on 2015-08-23

Fix leak and add documentation to readme.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:156
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sanity-tests-ci/690/
Executed test runs:
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sanity-tests-wily-amd64-ci/418
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sanity-tests-wily-armhf-ci/419
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sanity-tests-wily-i386-ci/418

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sanity-tests-ci/690/rebuild

review: Approve (continuous-integration)

[Max Brustkern (nuclearbob)]

Running without human interaction is my main use case. As an aside, the idea of the test runner altering it's config file every run is fairly concerning to me anyway; it seems like it could introduce race conditions when more than one instance runs simultaneously. Maybe this needs to be a bigger change discussed with more of the team?

[Sergio Cazzolato (sergio-j-cazzolato)]

Currently you are able to pass a config file as parameter for the tests execution.

What do you think about of pre-create all the config files that you need and then invoque the tests using the config file that you need?

Another alternative is that the test runner creates on the fly the config file and pass it as parameter to the test.

[Max Brustkern (nuclearbob)]

I think this should work well.

review: Approve

[Christopher Lee (veebers)]

Just a couple of minor things I came across when I was poking around.

review: Needs Fixing

[platform-qa-bot (platform-qa-bot)]

FAILED: Continuous integration, rev:156
https://platform-qa-jenkins.ubuntu.com/job/ubuntu-system-tests-ci/10/
Executed test runs:
    FAILURE: https://platform-qa-jenkins.ubuntu.com/job/build-wily-amd64-package/32/console
    FAILURE: https://platform-qa-jenkins.ubuntu.com/job/build-wily-i386-package/33/console
    None: https://platform-qa-jenkins.ubuntu.com/job/generic-update-mp/37/console

Click here to trigger a rebuild:
https://platform-qa-jenkins.ubuntu.com/job/ubuntu-system-tests-ci/10/rebuild

review: Needs Fixing (continuous-integration)

[platform-qa-bot (platform-qa-bot)]

FAILED: Continuous integration, rev:156
https://platform-qa-jenkins.ubuntu.com/job/ubuntu-system-tests-ci/1106/
Executed test runs:
    FAILURE: https://platform-qa-jenkins.ubuntu.com/job/build-xenial-amd64-package/352/console
    FAILURE: https://platform-qa-jenkins.ubuntu.com/job/build-xenial-i386-package/350/console
    None: https://platform-qa-jenkins.ubuntu.com/job/generic-update-mp/1828/console

Click here to trigger a rebuild:
https://platform-qa-jenkins.ubuntu.com/job/ubuntu-system-tests-ci/1106/rebuild

review: Needs Fixing (continuous-integration)

[platform-qa-bot (platform-qa-bot)]

FAILED: Continuous integration, rev:156
https://platform-qa-jenkins.ubuntu.com/job/ubuntu-system-tests-ci/1144/
Executed test runs:
    None: https://platform-qa-jenkins.ubuntu.com/job/generic-update-mp/1919/console

Click here to trigger a rebuild:
https://platform-qa-jenkins.ubuntu.com/job/ubuntu-system-tests-ci/1144/rebuild

review: Needs Fixing (continuous-integration)

Unmerged revisions

156. By Sergio Cazzolato on 2015-08-23

Fix leak and add documentation to readme.

155. By Sergio Cazzolato on 2015-08-23

Fixing unit tests

154. By Sergio Cazzolato on 2015-08-23

Fix to make possible to use temp config file for regular config data

153. By Sergio Cazzolato on 2015-08-21

Adding fix to delete the config files just when these are temporal

152. By Max Brustkern on 2015-08-13

Don't delete config path

151. By Max Brustkern on 2015-08-07

Cleaning up temp files

150. By Max Brustkern on 2015-07-29

Remove remaining use of hardcoded /tmp file on host

149. By Max Brustkern on 2015-07-15

Added non-static config file on host

148. By Max Brustkern on 2015-07-14

Separate host and device config file paths

=== modified file 'README.rst'
--- README.rst	2015-07-08 10:43:02 +0000
+++ README.rst	2015-08-23 04:04:32 +0000
@@ -82,14 +82,16 @@
   test.option1=test value 1
   test.option2=test value 2
 
-The default file is *~/.config/ubuntu-system-tests.conf*, but you can use a
-different file by passing it to the run command. See the `Run` section for more
-details.
+By default, if the file *~/.config/ubuntu-system-tests.conf* is created, it
+will be used, but you can use a different file by passing it to the run
+command. See the `Run` section for more details.
 
-If you don't have that file created the first time you run the tests, you will
-be prompted to enter values for all the options defined as mandatory. The
-values you provide in this interactive wizard will be saved to the conf file
-so they are kept for the following runs.
+If you don't have the config file created, a temporary config file will be
+used to run the tests (allowing to run more than once test execution in
+parallel fron the host machine). You will be prompted to enter values for all
+the options defined as mandatory. The values you provide in this interactive
+wizard will be used to run the current execution but won't be saved to any
+conf file.
 
 Copy data files to test server
 ------------------------------

=== modified file 'debian/tests/control'
--- debian/tests/control	2015-08-18 16:20:29 +0000
+++ debian/tests/control	2015-08-23 04:04:32 +0000
@@ -7,7 +7,7 @@
          gallery-app-autopilot,
          mediaplayer-app-autopilot,
          messaging-app-autopilot,
-         oxideqt-chromedriver,
+         #oxideqt-chromedriver,
          python3-autopilot,
          python3-dbus,
          python3-debian,
@@ -15,7 +15,7 @@
          python3-paramiko,
          python3-requests,
          python3-retrying,
-         python3-selenium,
+         #python3-selenium,
          python3-testtools,
          python3-warlock,
          subunit,

=== modified file 'debian/tests/systemtests'
--- debian/tests/systemtests	2015-07-01 05:13:40 +0000
+++ debian/tests/systemtests	2015-08-23 04:04:32 +0000
@@ -20,6 +20,8 @@
 
 import sys
 
+sys.path.append('.')
+
 from ubuntu_system_tests import systemtests
 
 

=== modified file 'ubuntu_system_tests/command_line.py'
--- ubuntu_system_tests/command_line.py	2015-08-11 09:26:08 +0000
+++ ubuntu_system_tests/command_line.py	2015-08-23 04:04:32 +0000
@@ -53,7 +53,7 @@
     parser = ArgumentParser('Run system tests for Ubuntu Touch.')
     parser.add_argument('--config', '-c',
                         help='The config file to use for this run.',
-                        default='ubuntu-system-tests.conf')
+                        default=None)
     parser.add_argument(
         '--update-apt', '-u', action='store_true',
         help='Update apt lists before installing the dependencies',
@@ -86,7 +86,6 @@
 
     enable_autopilot_introspection(config_stack)
     return_code = run_autopkg_test(config_stack, update_apt, network_ready)
-    config.delete_temporal_config_file()
 
     if return_code <= 6:
         # Return codes higher than 6 mean that there was a problem with adt or
@@ -135,7 +134,7 @@
     """
     config.register_test_options()
 
-    config_stack = config.UbuntuSystemTestsStack(conf_name)
+    config_stack = config.get_host_config_stack(conf_name)
 
     # We set the values passed from the command line to send them from the host
     # to the testbed.
@@ -280,9 +279,9 @@
 
 def _add_copy_to_temp_config_file(run_cmd):
     """ Add the copy parameter for the temp config file to the command line"""
-    if os.path.isfile(config.get_device_config_temp_file_path()):
+    if os.path.isfile(config.get_host_config_temp_file_path()):
         run_cmd.extend(['--copy', '{src}:{dest}'.format(
-            src=config.get_device_config_temp_file_path(),
+            src=config.get_host_config_temp_file_path(),
             dest=config.get_device_config_temp_file_path())])
 
 

=== modified file 'ubuntu_system_tests/config.py'
--- ubuntu_system_tests/config.py	2015-08-11 15:25:10 +0000
+++ ubuntu_system_tests/config.py	2015-08-23 04:04:32 +0000
@@ -21,6 +21,8 @@
 import logging
 import os
 
+from tempfile import mkstemp
+
 from uciconfig import (
     options,
     stacks,
@@ -126,33 +128,46 @@
     return '/tmp/ubuntu-system-tests.conf'
 
 
+def get_host_config_temp_file_path():
+    """Return the path of the config file with temporal data copied to the
+    device."""
+    global config_stack
+    if not config_stack:
+        return ''
+    return get_host_config_stack().host_temp_config_file
+
+
 def get_device_config_temp_file_path():
     """Return the path of the config file with temporal data copied to the
     device."""
     return '/tmp/ubuntu-system-tests-temp.conf'
 
 
-def get_config_stack_from_file(file_path):
-    """Return config stack from specified file path."""
-    return UbuntuSystemTestsStack(file_path)
-
-
 def get_device_config_stack():
-    """Return the config stack variable"""
+    """Return the config stack variable in the device"""
     global config_stack
     if not config_stack:
-        config_stack = get_config_stack_from_file(
-            get_device_config_file_path())
+        config_stack = \
+            UbuntuSystemTestsStack(get_device_config_file_path(),
+                                   get_device_config_temp_file_path())
         _add_temp_values_to_config_stack(config_stack)
     return config_stack
 
 
+def get_host_config_stack(config_name=None, temp_path=None):
+    """Return the config stack variable in the host machine"""
+    global config_stack
+    if not config_stack:
+        config_stack = UbuntuSystemTestsStack(config_name, temp_path)
+    return config_stack
+
+
 def _add_temp_values_to_config_stack(stack):
     """ Add the temporal data to the config stack. The temporal data is
     retrieved from the temporal config file which is deleted after the values
     are read
     """
-    tmp_tuples = _get_temp_config_values()
+    tmp_tuples = _get_temp_config_values(stack.host_temp_config_file)
 
     try:
         for pair in tmp_tuples:
@@ -162,12 +177,12 @@
         logger.error("Incorrect format for temporal variables.")
 
 
-def _get_temp_config_values(delete=True):
+def _get_temp_config_values(temp_path):
     """ Read the the temporal config values and delete the temp file if it is
     requested.
     """
-    if os.path.isfile(get_device_config_temp_file_path()):
-        with open(get_device_config_temp_file_path(), 'r') as f:
+    if os.path.isfile(temp_path):
+        with open(temp_path, 'r') as f:
             data = f.read()
 
         return data.split(',')
@@ -180,38 +195,72 @@
     the test execution
     :param temp_items: A dict with the items to store
     """
-    delete_temporal_config_file()
-
-    with open(get_device_config_temp_file_path(), 'w') as file:
+    with open(get_host_config_temp_file_path(), 'w') as file:
         for item, value in temp_items.items():
             file.write('{item}={value}'.format(item=item, value=value))
 
 
-def delete_temporal_config_file():
-    """Delete the temporal config stack file"""
-    if os.path.isfile(get_device_config_temp_file_path()):
-        os.remove(get_device_config_temp_file_path())
-
-
 def get_test_config_values(config_stack):
     """Returns a string containing the comma separated key=value options.
     The list includes the cofig values and the temporal ones as well"""
     values = config_stack.get_autopilot_config_string()
-    tmp_vars = _get_temp_config_values()
+    tmp_vars = _get_temp_config_values(config_stack.host_temp_config_file)
 
     return '{values},{temp}'.format(values=values, temp=tmp_vars)
 
 
 class UbuntuSystemTestsStack(stacks.Stack):
 
-    def __init__(self, conf_name='ubuntu-system-tests.conf'):
-        config_path = os.path.join(
-            get_user_config_dir(), conf_name)
-        user_store = self.get_shared_store(UbuntuSystemTestsStore(config_path))
+    DEFAULT_CONFIG = 'ubuntu-system-tests.conf'
+
+    def __init__(self, config_name=None, temp_path=None):
+
+        # If either an specific config name hs been requested or the default
+        # config file already exists, set it as config file
+        self.host_config_path = self._get_predefined_config_path(config_name)
+        if not self.host_config_path:
+            self.host_config_path = mkstemp()[1]
+
+        # Initialize the super
+        user_store = self.get_shared_store(UbuntuSystemTestsStore(
+            self.host_config_path))
         super().__init__(
             [stacks.NameMatcher(user_store, None).get_sections],
             user_store, mutable_section_id=None)
+
+        # Create the stack and config file for temporal data
         self.temp_stack = dict()
+        if temp_path:
+            self.host_temp_config_file = temp_path
+        else:
+            self.host_temp_config_file = mkstemp()[1]
+
+    def __del__(self):
+        # just temporal files created with mkstemp will be deleted.
+        self._del_config_file(self.host_temp_config_file)
+        self._del_config_file(self.host_config_path)
+
+    def _del_config_file(self, config_path):
+        """ Delete the config file when it is not created in the default
+        config dir
+        """
+        if os.path.exists(config_path) and \
+            os.path.dirname(config_path) != get_user_config_dir():
+            os.unlink(config_path)
+
+    def _get_predefined_config_path(self, config_name):
+        """ Get the config file path in the default dir """
+
+        # When the config name is set, the config will be stored in there.
+        if config_name:
+            return os.path.join(get_user_config_dir(), config_name)
+
+        # If the default file is created, this will be used
+        elif os.path.exists(os.path.join(get_user_config_dir(),
+                                         self.DEFAULT_CONFIG)):
+            return os.path.join(get_user_config_dir(), self.DEFAULT_CONFIG)
+
+        return None
 
     def get_missing_options_from_stdin(self):
         """Ask the user for the values of the missing options.

=== modified file 'ubuntu_system_tests/selftests/test_command_line.py'
--- ubuntu_system_tests/selftests/test_command_line.py	2015-06-30 11:21:23 +0000
+++ ubuntu_system_tests/selftests/test_command_line.py	2015-08-23 04:04:32 +0000
@@ -172,16 +172,16 @@
         self.set_config_values(device_serial='')
 
         config_stack = command_line._get_config_stack(
-            'ubuntu-system-tests.conf', tests_to_run=[])
+            self.config_stack.host_config_path, tests_to_run=[])
 
         self.assertEqual('', config_stack.get('tests_to_run'))
 
     def test_get_config_stack_with_tests_to_run(self):
         self.set_config_values(device_serial='')
 
-        config_stack = command_line._get_config_stack(
-            'ubuntu-system-tests.conf',
-            tests_to_run=['test1', 'test2', 'test3'])
+        config_stack = command_line.\
+            _get_config_stack(self.config_stack.host_config_path,
+                              tests_to_run=['test1', 'test2', 'test3'])
 
         self.assertEqual(
             'test1 test2 test3', config_stack.get('tests_to_run'))
@@ -252,6 +252,7 @@
             temp_control = command_line._create_temp_control_file(
                 ['dep4', 'dep5'], control.name
             )
+            self.addCleanup(os.remove, temp_control)
             with open(temp_control) as temp:
                 tmp_control_content = temp.read()
                 expected_content = """Tests: systemtests
@@ -320,7 +321,7 @@
 
 class MainPassingCommandLineArgumentsTestCase(testscenarios.TestWithScenarios):
 
-    DEFAULT_CONFIG_FILE = 'ubuntu-system-tests.conf'
+    DEFAULT_CONFIG_FILE = None
     RUN_ALL_TESTS = []
     DO_APT_UPDATE = True
     DONT_APT_UPDATE = False

=== modified file 'ubuntu_system_tests/selftests/test_config.py'
--- ubuntu_system_tests/selftests/test_config.py	2015-05-21 18:15:40 +0000
+++ ubuntu_system_tests/selftests/test_config.py	2015-08-23 04:04:32 +0000
@@ -33,7 +33,7 @@
 from ubuntu_system_tests.helpers.autopilot import override_proxy_timeout
 
 
-class HelpersTestCase(selftests.ConfigBaseTestCase):
+class HelpersTest7Case(selftests.ConfigBaseTestCase):
 
     def get_mock_proxy(self):
         """Returns a rubbish proxy object that doesn't actually do anything."""
@@ -112,10 +112,10 @@
 class UbuntuSystemTestsStackTestCase(selftests.ConfigBaseTestCase):
 
     def test_store_location(self):
-        config_stack = config.UbuntuSystemTestsStack()
+        config_stack = config.UbuntuSystemTestsStack('tests.conf')
 
         expected_config_path = os.path.join(
-            os.environ.get('HOME'), '.config', 'ubuntu-system-tests.conf')
+            os.environ.get('HOME'), '.config', 'tests.conf')
         self.assertEqual(expected_config_path, config_stack.store.path)
 
     def test_get_autopilot_config_string(self):

=== modified file 'ubuntu_system_tests/systemtests.py'
--- ubuntu_system_tests/systemtests.py	2015-07-15 17:03:36 +0000
+++ ubuntu_system_tests/systemtests.py	2015-08-23 04:04:32 +0000
@@ -103,7 +103,8 @@
 
 def ensure_temp_stack_deleted():
     """Delete the temporal config file after the tests are executed"""
-    config.delete_temporal_config_file()
+    if os.path.exists(config.get_device_config_temp_file_path()):
+        os.remove(config.get_device_config_temp_file_path())
 
 
 if __name__ == '__main__':