Merge lp:~lifeless/subunit/tags into lp:~subunit/subunit/trunk

Proposed by Robert Collins
Status: Merged
Merged at revision: not available
Proposed branch: lp:~lifeless/subunit/tags
Merge into: lp:~subunit/subunit/trunk
To merge this branch: bzr merge lp:~lifeless/subunit/tags
Reviewer Review Type Date Requested Status
Jonathan Lange Needs Fixing
Review via email: mp+2186@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote :

Support for tags in the server, allowing clients to define tags for all tests or a single test.

Revision history for this message
Jonathan Lange (jml) wrote :
Download full text (5.8 KiB)

=== modified file 'README'
--- README 2007-01-17 11:39:01 +0000
+++ README 2008-12-12 01:11:46 +0000
@@ -156,8 +156,14 @@
 error: test label
 error: test label [
 ]
+tags: [-]TAG ...
 unexpected output on stdout -> stdout.
 exit w/0 or last test -> error
+tags given outside a test are applied to all following tests
+tags given within a test inherit the current global tags
+
+In python, tags are assigned to the .tags attribute on the RemoteTest objects
+created by the TestProtocolServer.

 TODO:
 def run:

=== modified file 'python/subunit/__init__.py'
--- python/subunit/__init__.py 2007-02-12 19:29:18 +0000
+++ python/subunit/__init__.py 2008-12-12 01:11:46 +0000
@@ -43,7 +43,10 @@

 class TestProtocolServer(object):
- """A class for receiving results from a TestProtocol client."""
+ """A class for receiving results from a TestProtocol client.
+
+ :ivar tags: The current tags associated with the protocol stream.
+ """

     OUTSIDE_TEST = 0
     TEST_STARTED = 1
@@ -64,6 +67,7 @@
         self.state = TestProtocolServer.OUTSIDE_TEST
         self.client = client
         self._stream = stream
+ self.tags = set()

     def _addError(self, offset, line):
         if (self.state == TestProtocolServer.TEST_STARTED and
@@ -128,6 +132,23 @@
         else:
             self.stdOutLineReceived(line)

+ def _handleTags(self, offset, line):
+ """Process a tags command."""
+ tags = line[offset:].split()
+ new_tags = set()
+ gone_tags = set()
+ for tag in tags:
+ if tag[0] == '-':
+ gone_tags.add(tag[1:])
+ else:
+ new_tags.add(tag)
+ if self.state == TestProtocolServer.OUTSIDE_TEST:
+ update_tags = self.tags
+ else:
+ update_tags = self._current_test.tags
+ update_tags.update(new_tags)
+ update_tags.difference_update(gone_tags)
+
     def lineReceived(self, line):
         """Call the appropriate local method for the received line."""
         if line == "]\n":
@@ -149,6 +170,8 @@
                     self._addFailure(offset, line)
                 elif cmd in ('success', 'successful'):
                     self._addSuccess(offset, line)
+ elif cmd in ('tags'):
+ self._handleTags(offset, line)
                 else:
                     self.stdOutLineReceived(line)
             else:
@@ -188,6 +211,7 @@
             self._current_test = RemotedTestCase(line[offset:-1])
             self.current_test_description = line[offset:-1]
             self.client.startTest(self._current_test)
+ self._current_test.tags = set(self.tags)
         else:
             self.stdOutLineReceived(line)

@@ -242,7 +266,14 @@

 class RemotedTestCase(unittest.TestCase):
- """A class to represent test cases run in child processes."""
+ """A class to represent test cases run in child processes.
+
+ Instances of this class are used to provide the python test API a TestCase
+ that can be printed to the screen, introspected for metadata and so on.
+ However, as they are a simply a memoisation of a test that was actually
+ run in the past by a s...

Read more...

Revision history for this message
Jonathan Lange (jml) wrote :
Download full text (4.5 KiB)

On Sun, Dec 7, 2008 at 7:00 AM, Robert Collins
<email address hidden> wrote:
> Robert Collins has proposed merging lp:~lifeless/subunit/tags into lp:subunit.
>
> Requested reviews:
> Jonathan Lange (jml)
>
> Support for tags in the server, allowing clients to define tags for all tests or a single test.

Hi Rob,

I approve of the feature, the tests look good and the code doesn't
have any obvious bugs. I have a couple of minor spelling comments and
a request for more documentation.

  vote needs_fixing

On Fri, Dec 12, 2008 at 12:17 PM, Jonathan Lange <email address hidden> wrote:
> === modified file 'README'
> --- README 2007-01-17 11:39:01 +0000
> +++ README 2008-12-12 01:11:46 +0000
> @@ -156,8 +156,14 @@
> error: test label
> error: test label [
> ]
> +tags: [-]TAG ...
> unexpected output on stdout -> stdout.
> exit w/0 or last test -> error
> +tags given outside a test are applied to all following tests
> +tags given within a test inherit the current global tags
> +
> +In python, tags are assigned to the .tags attribute on the RemoteTest objects

"Python", not "python"

> +created by the TestProtocolServer.
>

The README should explain how -TAG works. Also, looking at the
protocol description, it's not clear what "within a test" means. I
guess that's "after a 'test' message"?

> TODO:
> def run:
>
> === modified file 'python/subunit/__init__.py'
> --- python/subunit/__init__.py 2007-02-12 19:29:18 +0000
> +++ python/subunit/__init__.py 2008-12-12 01:11:46 +0000
> @@ -43,7 +43,10 @@
>
>
> class TestProtocolServer(object):
> - """A class for receiving results from a TestProtocol client."""
> + """A class for receiving results from a TestProtocol client.
> +
> + :ivar tags: The current tags associated with the protocol stream.
> + """
>
> OUTSIDE_TEST = 0
> TEST_STARTED = 1
> @@ -64,6 +67,7 @@
> self.state = TestProtocolServer.OUTSIDE_TEST
> self.client = client
> self._stream = stream
> + self.tags = set()
>
> def _addError(self, offset, line):
> if (self.state == TestProtocolServer.TEST_STARTED and
> @@ -128,6 +132,23 @@
> else:
> self.stdOutLineReceived(line)
>
> + def _handleTags(self, offset, line):
> + """Process a tags command."""
> + tags = line[offset:].split()
> + new_tags = set()
> + gone_tags = set()
> + for tag in tags:
> + if tag[0] == '-':
> + gone_tags.add(tag[1:])
> + else:
> + new_tags.add(tag)

This could also be written as:
    tags = set(line[offset:].split())
    gone_tags = set(tag for tag in tags if tag[0] == '-')
    new_tags = tags - gone_tags

I mildly prefer that style, but that's purely a personal taste thing. Your call.

(Python should have a "dichotomy" function for sets.)

> + if self.state == TestProtocolServer.OUTSIDE_TEST:
> + update_tags = self.tags
> + else:
> + update_tags = self._current_test.tags
> + update_tags.update(new_tags)
> + update_tags.difference_update(gone_tags)
> +
> def lineReceived(self, line):
> """Call the appropriate local method for t...

Read more...

review: Needs Fixing
lp:~lifeless/subunit/tags updated
51. By Robert Collins

Review feedback from jml.

Subscribers

People subscribed via source and target branches