Autopilot

Merge lp:~veebers/autopilot/fix_for_config_init into lp:autopilot

fix_for_config_init
Merge into trunk

Proposed by Christopher Lee on 2015-03-05

Status:

Merged

Approved by:

Christopher Lee on 2015-03-12

Approved revision:

547

Merged at revision:

545

Proposed branch:

lp:~veebers/autopilot/fix_for_config_init

Merge into:

lp:autopilot

Diff against target:

74 lines (+36/-10)

2 files modified

autopilot/run.py (+11/-10)
autopilot/tests/functional/test_autopilot_functional.py (+25/-0)

To merge this branch:

bzr merge lp:~veebers/autopilot/fix_for_config_init

Medium

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot	continuous-integration		Approve on 2015-03-11
Richard Huddie (community)			Approve on 2015-03-11
Leo Arias		2015-03-05	Approve on 2015-03-10
Review via email: mp+251862@code.launchpad.net

Commit Message

Move config parsing to happen before test import.

Description of the Change

Move config parsing to happen before test import. Ensures config details available within decorators.

PS Jenkins bot (ps-jenkins) wrote on 2015-03-05:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/autopilot-ci/1017/rebuild

review: Approve (continuous-integration)

Leo Arias (elopio) wrote on 2015-03-09:

This looks good. Can we write a test for it?
I think that using a mock and checking the order of the calls would be good.
Even better if we add a skip decorator in a test that uses a config value.

lp:~veebers/autopilot/fix_for_config_init updated on 2015-03-10

546. By Christopher Lee on 2015-03-10: Failing test for config available in decorator
547. By Christopher Lee on 2015-03-10: test is passing (no functional code change, demos that config now available)

Christopher Lee (veebers) wrote on 2015-03-10:

@Leo, I've added a first pass at a functional test for this as requested.

I'm not son keen on doing a test using mocks etc. to check the order of the calls. The calls are static and we can see them in the source file (e.g. they aren't conditionally run etc.) as well as the whole suite of tests that cover the execution of this part of the code which should give us coverage already.

PS Jenkins bot (ps-jenkins) wrote on 2015-03-10:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/autopilot-ci/1018/rebuild

review: Needs Fixing (continuous-integration)

Leo Arias (elopio) wrote on 2015-03-10:

Without the fix the test fails. With it the test passes.
I actually prefer this to be tested in the functional suite, so +2.

review: Approve

PS Jenkins bot (ps-jenkins) wrote on 2015-03-10:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/autopilot-ci/1019/rebuild

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2015-03-10:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/autopilot-ci/1020/rebuild

review: Needs Fixing (continuous-integration)

Richard Huddie (rhuddie) wrote on 2015-03-11:

This all looks good, very interesting how the new test case is done. Test passes for me.

review: Approve

Leo Arias (elopio) wrote on 2015-03-11:

Asking ci again about the tests. They are stuck.

PS Jenkins bot (ps-jenkins) wrote on 2015-03-11:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/autopilot-ci/1021/rebuild

review: Approve (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2015-03-11:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/autopilot-ci/1022/rebuild

review: Approve (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2015-03-11:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/autopilot-ci/1023/rebuild

review: Approve (continuous-integration)

Christopher Lee (veebers) wrote on 2015-03-12:

Top approving as it has the required 2x approves and now that CI is resolved they are passing.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Autopilot Hackers

Chris Gagnon

Christopher Lee

 === modified file 'autopilot/run.py'
 --- autopilot/run.py	2014-09-05 04:55:50 +0000
 +++ autopilot/run.py	2015-03-10 00:23:59 +0000
@@ -662,16 +662,6 @@
      def run_tests(self):
          """Run tests, using input from `args`."""
--        test_suite, error_encountered = load_test_suite_from_name(
--            self.args.suite
--        )
--
--        if not test_suite.countTestCases():
--            raise RuntimeError('Did not find any tests')
--
--        if self.args.random_order:
--            shuffle(test_suite._tests)
--            print("Running tests in random order")
          _configure_debug_profile(self.args)
          _configure_timeout_profile(self.args)
@@ -688,6 +678,17 @@
          if self.args.verbose:
              autopilot.globals.set_log_verbose(True)
++        test_suite, error_encountered = load_test_suite_from_name(
++            self.args.suite
++        )
++
++        if not test_suite.countTestCases():
++            raise RuntimeError('Did not find any tests')
++
++        if self.args.random_order:
++            shuffle(test_suite._tests)
++            print("Running tests in random order")
++
          result = construct_test_result(self.args)
          result.startTestRun()
          try:
 === modified file 'autopilot/tests/functional/test_autopilot_functional.py'
 --- autopilot/tests/functional/test_autopilot_functional.py	2014-07-24 00:24:41 +0000
 +++ autopilot/tests/functional/test_autopilot_functional.py	2015-03-10 00:23:59 +0000
@@ -94,6 +94,31 @@
  class FunctionalTestMain(AutopilotFunctionalTestsBase):
++    def test_config_available_in_decorator(self):
++        """Any commandline config values must be available for decorators."""
++        unique_config_value = self.getUniqueString()
++        self.create_test_file(
++            'test_config_skip.py', dedent("""\
++            from testtools import skipIf
++            import autopilot
++            from autopilot.testcase import AutopilotTestCase
++
++            class ConfigTest(AutopilotTestCase):
++                @skipIf(
++                    autopilot.get_test_configuration().get('skipme', None)
++                    == '{unique_config_value}',
++                    'Skipping Test')
++                def test_config(self):
++                    self.fail('Should not run.')
++            """.format(unique_config_value=unique_config_value))
++        )
++
++        config_string = 'skipme={}'.format(unique_config_value)
++        code, output, error = self.run_autopilot(
++            ['run', 'tests', '--config', config_string])
++
++        self.assertThat(code, Equals(0))
++
      def test_can_list_empty_test_dir(self):
          """Autopilot list must report 0 tests found with an empty test
          module."""