Merge lp:~allenap/launchpad/id-for-yui-test-cases into lp:launchpad

Proposed by Gavin Panella
Status: Merged
Approved by: Gavin Panella
Approved revision: no longer in the source branch.
Merged at revision: 12639
Proposed branch: lp:~allenap/launchpad/id-for-yui-test-cases
Merge into: lp:launchpad
Diff against target: 71 lines (+25/-5)
2 files modified
lib/lp/testing/__init__.py (+18/-1)
lib/lp/testing/tests/test_testing.py (+7/-4)
To merge this branch: bzr merge lp:~allenap/launchpad/id-for-yui-test-cases
Reviewer Review Type Date Requested Status
Jonathan Lange (community) Approve
Review via email: mp+54229@code.launchpad.net

Commit message

[r=jml][no-qa] Implement YUIUnitTestCase.id() so that YUI tests have a distinctive identifier.

Description of the change

This branch fixes the following:

{{{
$ bin/test --layer WindmillLayer -t yuitest --list-tests
Listing lp.app.windmill.testing.AppWindmillLayer tests:
  lp.app.windmill.tests.test_yuitests.AppYUIUnitTestCase.runTest
  lp.app.windmill.tests.test_yuitests.AppYUIUnitTestCase.runTest
Listing lp.bugs.windmill.testing.BugsWindmillLayer tests:
  lp.bugs.windmill.tests.test_yuitests.BugsYUIUnitTestCase.runTest
  lp.bugs.windmill.tests.test_yuitests.BugsYUIUnitTestCase.runTest
Listing lp.code.windmill.testing.CodeWindmillLayer tests:
  lp.code.windmill.tests.test_yuitests.CodeYUIUnitTestCase.runTest
Listing lp.registry.windmill.testing.RegistryWindmillLayer tests:
  lp.registry.windmill.tests.test_yuitests.RegistryYUIUnitTestCase.runTest
  lp.registry.windmill.tests.test_yuitests.RegistryYUIUnitTestCase.runTest
  lp.registry.windmill.tests.test_yuitests.RegistryYUIUnitTestCase.runTest
Listing lp.soyuz.windmill.testing.SoyuzWindmillLayer tests:
  lp.soyuz.windmill.tests.test_yuitests.SoyuzYUIUnitTestCase.runTest
  lp.soyuz.windmill.tests.test_yuitests.SoyuzYUIUnitTestCase.runTest
}}}

to look more like:

{{{
$ bin/test --layer WindmillLayer -t javascript/tests --list-tests
Listing lp.app.windmill.testing.AppWindmillLayer tests:
  lp/app/javascript/tests/test_lp_collapsibles.html
  lp/app/javascript/tests/test_lp_client.html
Listing lp.bugs.windmill.testing.BugsWindmillLayer tests:
  lp/bugs/javascript/tests/test_me_too.html
  lp/bugs/javascript/tests/test_subscriber.html
Listing lp.code.windmill.testing.CodeWindmillLayer tests:
  lp/code/javascript/tests/test_productseries-setbranch.html
Listing lp.registry.windmill.testing.RegistryWindmillLayer tests:
  lp/registry/javascript/tests/test_milestone_table.html
  lp/registry/javascript/tests/test_timeline.html
Listing lp.soyuz.windmill.testing.SoyuzWindmillLayer tests:
  lp/soyuz/javascript/tests/test_archivesubscribers_index.html
  lp/soyuz/javascript/tests/test_lp_dynamic_dom_updater.html
}}}

To post a comment you must log in.
Revision history for this message
Jonathan Lange (jml) wrote :

<jml> allenap: Looks basically sane to me. Two things come to mind.
<jml> allenap: first is that you should add a test for this.
<jml> allenap: second is that the ideal id() allows you to load the test based on the id. I don't know enough about how JS tests are run to know if that's the case here. It seems likely though.
<allenap> jml: Fair. This whole class is untested so I was hoping to sneak this one through too.
<jml> allenap: self.assertEqual(test.test_path, test.id())
<jml> allenap: there you go, I've written the test for you :P
<allenap> jml: Cheers :)
<jml> also, in random spam news, I may be entitled to receive £3750 for the accident I had. (I didn't have an accident)
<jml> allenap: looks like a very useful change to make

I would have made the ID for like foo.bar.baz than foo/bar/baz.html, but I think that's actually not necessary, and maybe not even universally preferable. The general principles are uniqueness and being able to load the test based on the id.

Thanks Gavin.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/testing/__init__.py'
--- lib/lp/testing/__init__.py 2011-03-21 09:15:49 +0000
+++ lib/lp/testing/__init__.py 2011-03-21 16:08:30 +0000
@@ -840,9 +840,21 @@
840840
841 _yui_results = None841 _yui_results = None
842842
843 def __init__(self):
844 """Create a new test case without a choice of test method name.
845
846 Preventing the choice of test method ensures that we can safely
847 provide a test ID based on the file path.
848 """
849 super(YUIUnitTestCase, self).__init__("checkResults")
850
843 def initialize(self, test_path):851 def initialize(self, test_path):
844 self.test_path = test_path852 self.test_path = test_path
845853
854 def id(self):
855 """Return an ID for this test based on the file path."""
856 return self.test_path
857
846 def setUp(self):858 def setUp(self):
847 super(YUIUnitTestCase, self).setUp()859 super(YUIUnitTestCase, self).setUp()
848 #This goes here to prevent circular import issues860 #This goes here to prevent circular import issues
@@ -874,7 +886,12 @@
874 self._yui_results[test_name] = dict(886 self._yui_results[test_name] = dict(
875 result=result, message=message)887 result=result, message=message)
876888
877 def runTest(self):889 def checkResults(self):
890 """Check the results.
891
892 The tests are run during `setUp()`, but failures need to be reported
893 from here.
894 """
878 if self._yui_results is None or len(self._yui_results) == 0:895 if self._yui_results is None or len(self._yui_results) == 0:
879 self.fail("Test harness or js failed.")896 self.fail("Test harness or js failed.")
880 for test_name in self._yui_results:897 for test_name in self._yui_results:
881898
=== modified file 'lib/lp/testing/tests/test_testing.py'
--- lib/lp/testing/tests/test_testing.py 2010-10-26 15:47:24 +0000
+++ lib/lp/testing/tests/test_testing.py 2011-03-21 16:08:30 +0000
@@ -5,14 +5,13 @@
55
6__metaclass__ = type6__metaclass__ = type
77
8import unittest
9
10from canonical.testing.layers import DatabaseFunctionalLayer8from canonical.testing.layers import DatabaseFunctionalLayer
11from lp.services.features import getFeatureFlag9from lp.services.features import getFeatureFlag
12from lp.testing import (10from lp.testing import (
13 feature_flags,11 feature_flags,
14 set_feature_flag,12 set_feature_flag,
15 TestCase,13 TestCase,
14 YUIUnitTestCase,
16 )15 )
1716
1817
@@ -37,5 +36,9 @@
37 self.assertIs(None, getFeatureFlag('name'))36 self.assertIs(None, getFeatureFlag('name'))
3837
3938
40def test_suite():39class TestYUIUnitTestCase(TestCase):
41 return unittest.TestLoader().loadTestsFromName(__name__)40
41 def test_id(self):
42 test = YUIUnitTestCase()
43 test.initialize("foo/bar/baz.html")
44 self.assertEqual(test.test_path, test.id())