Skip to content

Commit

Permalink
work in progress - not sure if this is the best idea, as it might for…
Browse files Browse the repository at this point in the history
…ce the rest of buildbot to become properly unicode-aware
  • Loading branch information
Dustin J. Mitchell committed May 14, 2010
1 parent a675abc commit bb64620
Show file tree
Hide file tree
Showing 8 changed files with 197 additions and 62 deletions.
14 changes: 13 additions & 1 deletion buildbot/broken_test/runs/test_status.py
Expand Up @@ -153,7 +153,19 @@ def customTextMailMessage(attrs):
text = list()
text.append("STATUS: %s" % attrs['result'].title())
text.append("")
text.extend([c.asText() for c in attrs['changes']])
def ch2txt(c):
data = ""
data += c.getFileContents()
if c.repository:
data += "On: %s\n" % c.repository
if c.project:
data += "For: %s\n" % c.project
data += "At: %s\n" % c.getTime()
data += "Changed By: %s\n" % c.who
data += "Comments: %s" % c.comments
data += "Properties: \n%s\n\n" % c.getProperties()
return data
text.extend([ch2txt(c) for c in attrs['changes']])
text.append("")
name, url, lines, status = attrs['logs'][-1]
text.append("Last %d lines of '%s':" % (logLines, name))
Expand Down
75 changes: 27 additions & 48 deletions buildbot/changes/changes.py
Expand Up @@ -32,7 +32,11 @@ class Change:
Changes should be submitted to ChangeMaster.addChange() in
chronologically increasing order. Out-of-order changes will probably
cause the web status displays to be corrupted."""
cause the web status displays to be corrupted.
All strings used to create a new Change must be unicode objects. It is
up to the caller to decode bytestrings generated by whatever underlying
version-control system might have generated the information."""

implements(interfaces.IStatusEvent)

Expand All @@ -42,26 +46,25 @@ class Change:
category = None
revision = None # used to create a source-stamp

unicode_attrs = "who comments revlink category branch revision repository project".split()

def __init__(self, who, files, comments, isdir=0, links=None,
revision=None, when=None, branch=None, category=None,
revlink='', properties={}, repository='', project=''):
revlink=u'', properties={}, repository=u'', project=u'',
_skip_unicode_check=False):
self.who = who
self.comments = comments
self.isdir = isdir
if links is None:
links = []
self.links = links

def none_or_unicode(x):
if x is None: return x
return unicode(x)

self.revision = none_or_unicode(revision)
self.revision = revision
if when is None:
when = util.now()
self.when = when
self.branch = none_or_unicode(branch)
self.category = none_or_unicode(category)
self.branch = branch
self.category = category
self.revlink = revlink
self.properties = Properties()
self.properties.update(properties, "Change")
Expand All @@ -72,6 +75,15 @@ def none_or_unicode(x):
self.files = files[:]
self.files.sort()

# check that we got unicode characters as necessary
# automatic way - so this method is used before adding new Change objects
# to the database, to ensure that they are well-formed.
if _skip_unicode_check: return # for tests only!
for attr in self.unicode_attrs:
a = getattr(self, attr)
if a is not None and not isinstance(a, unicode):
raise ValueError("value of Change attribute '%s' is not unicode" % attr)

def __setstate__(self, dict):
self.__dict__ = dict
# Older Changes won't have a 'properties' attribute in them
Expand All @@ -80,19 +92,6 @@ def __setstate__(self, dict):
if not hasattr(self, 'revlink'):
self.revlink = ""

def asText(self):
data = ""
data += self.getFileContents()
if self.repository:
data += "On: %s\n" % self.repository
if self.project:
data += "For: %s\n" % self.project
data += "At: %s\n" % self.getTime()
data += "Changed By: %s\n" % self.who
data += "Comments: %s" % self.comments
data += "Properties: \n%s\n\n" % self.getProperties()
return data

def asDict(self):
'''returns a dictonary with suitable info for html/mail rendering'''
result = {}
Expand All @@ -112,7 +111,7 @@ def asDict(self):
result['number'] = self.number
result['branch'] = self.branch
result['category'] = self.category
result['who'] = self.getShortAuthor()
result['who'] = self.who
result['comments'] = self.comments
result['revision'] = self.revision
result['rev'] = self.revision
Expand All @@ -125,41 +124,21 @@ def asDict(self):
result['project'] = getattr(self, 'project', None)
return result

def getShortAuthor(self):
return self.who

def getTime(self):
"""Return a pretty-printed version of self.when"""
if not self.when:
return "?"
return time.strftime("%a %d %b %Y %H:%M:%S",
time.localtime(self.when))

## IStatusEvent methods

def getTimes(self):
"""IStatusEvent method, used by the waterfall"""
return (self.when, None)

def getText(self):
return [html.escape(self.who)]
def getLogs(self):
return {}

def getFileContents(self):
data = ""
if len(self.files) == 1:
if self.isdir:
data += "Directory: %s\n" % self.files[0]
else:
data += "File: %s\n" % self.files[0]
else:
data += "Files:\n"
for f in self.files:
data += " %s\n" % f
return data

def getProperties(self):
data = ""
for prop in self.properties.asList():
data += " %s: %s" % (prop[0], prop[1])
return data


class ChangeMaster:
Expand Down Expand Up @@ -204,7 +183,7 @@ def recode_changes(self, old_encoding, quiet=False):
if isinstance(c.revision, int):
c.revision = unicode(c.revision)

for attr in ("who", "comments", "revlink", "category", "branch", "revision"):
for attr in Change.unicode_attrs:
a = getattr(c, attr)
if isinstance(a, str):
try:
Expand Down
1 change: 1 addition & 0 deletions buildbot/changes/manager.py
Expand Up @@ -97,6 +97,7 @@ def removeSource(self, source):
def addChange(self, change):
"""Deliver a file change event. The event should be a Change object.
This method will timestamp the object as it is received."""
assert change.verifyIsUnicode(), "incoming Change contains non-unicode strings; see log"
log.msg("adding change, who %s, %d files, rev=%s, branch=%s, repository=%s, "
"comments %s, category %s" % (change.who, len(change.files),
change.revision, change.branch, change.repository,
Expand Down
2 changes: 1 addition & 1 deletion buildbot/interfaces.py
Expand Up @@ -732,7 +732,7 @@ def getText():
"""Returns a list of strings which describe the event. These are
intended to be displayed in a narrow column. If more space is
available, the caller should join them together with spaces before
presenting them to the user."""
presenting them to the user. The strings should be html-encoded."""


LOG_CHANNEL_STDOUT = 0
Expand Down
7 changes: 0 additions & 7 deletions buildbot/process/base.py
Expand Up @@ -111,13 +111,6 @@ def blamelist(self):
blamelist.sort()
return blamelist

def changesText(self):
changetext = ""
for c in self.allChanges():
changetext += "-" * 60 + "\n\n" + c.asText() + "\n"
# consider sorting these by number
return changetext

def setStepFactories(self, step_factories):
"""Set a list of 'step factories', which are tuples of (class,
kwargs), where 'class' is generally a subclass of step.BuildStep .
Expand Down
2 changes: 1 addition & 1 deletion buildbot/status/web/changes.py
Expand Up @@ -44,7 +44,7 @@ def getBox(self, req):
url = req.childLink("../changes/%d" % self.original.number)
template = req.site.buildbot_service.templates.get_template("change_macros.html")
text = template.module.box_contents(url=url,
who=self.original.getShortAuthor(),
who=self.original.who,
title=self.original.comments)
return Box([text], class_="Change")
components.registerAdapter(ChangeBox, Change, IBox)
Expand Down
8 changes: 4 additions & 4 deletions buildbot/test/regressions/test_import_unicode_changes.py
Expand Up @@ -38,7 +38,7 @@ def testUnicodeChange(self):
# Create changes.pck
changes = [Change(who=u"Frosty the \N{SNOWMAN}".encode("utf8"),
files=["foo"], comments=u"Frosty the \N{SNOWMAN}".encode("utf8"),
branch="b1", revision=12345)]
branch="b1", revision=12345, _skip_unicode_check=1)]
cPickle.dump(Thing(changes=changes), open(os.path.join(self.basedir,
"changes.pck"), "w"))

Expand All @@ -53,7 +53,7 @@ def testUnicodeChange(self):
def testNonUnicodeChange(self):
# Create changes.pck
changes = [Change(who="\xff\xff\x00", files=["foo"],
comments="\xff\xff\x00", branch="b1", revision=12345)]
comments="\xff\xff\x00", branch="b1", revision=12345, _skip_unicode_check=1)]
cPickle.dump(Thing(changes=changes), open(os.path.join(self.basedir,
"changes.pck"), "w"))

Expand All @@ -63,7 +63,7 @@ def testNonUnicodeChange(self):
def testAsciiChange(self):
# Create changes.pck
changes = [Change(who="Frosty the Snowman",
files=["foo"], comments="Frosty the Snowman", branch="b1", revision=12345)]
files=["foo"], comments="Frosty the Snowman", branch="b1", revision=12345, _skip_unicode_check=1)]
cPickle.dump(Thing(changes=changes), open(os.path.join(self.basedir,
"changes.pck"), "w"))

Expand All @@ -80,7 +80,7 @@ def testUTF16Change(self):
cm = OldChangeMaster()
cm.changes = [Change(who=u"Frosty the \N{SNOWMAN}".encode("utf16"),
files=["foo"], comments=u"Frosty the \N{SNOWMAN}".encode("utf16"),
branch="b1", revision=12345)]
branch="b1", revision=12345, _skip_unicode_check=1)]

# instead of running contrib/fix_changes_pickle_encoding.py, we just call
# the changemanager's recode_changes directly - it's the function at the
Expand Down
150 changes: 150 additions & 0 deletions buildbot/test/unit/test_changes_changes.py
@@ -0,0 +1,150 @@
import time

from twisted.trial import unittest

from buildbot.changes import changes

class Change(unittest.TestCase):
def assertEqualUnicode(self, x, y, msg=None):
if not isinstance(x, unicode):
self.fail("%r is not unicode" % (x,))
self.assertEqual(x, y, msg)

def getBigChange(self):
return changes.Change(u'me', [ u'foo.c' ], u'fixes',
isdir=1, links=['http://buildbot.net'],
revision=u'456', when=1273727073, branch=u'branches/release',
category=u'important', revlink=u'http://buildbot.net',
properties={'foo':'bar'}, repository=u'http://svn.buildbot.net',
project=u'buildbot')

# --

def test_constructor_minimal(self):
c = changes.Change(u'djmitche', [ u'changes.py' ], u'I hate this file')
self.assertEqual(
[ c.who, c.files, c.comments,
c.isdir, c.links, c.revision, # note: skip c.when
c.branch, c.category,
c.revlink, c.properties.asList(),
c.repository, c.project ],
[ u'djmitche', [ u'changes.py' ], u'I hate this file',
0, [], None,
None, None,
'', [],
'', ''])

def test_constructor_isdir(self):
c = changes.Change(u'me', [ u'foo.c' ], u'fixes',
isdir=1)
self.assertEqual(c.isdir, 1)

def test_constructor_links(self):
c = changes.Change(u'me', [ u'foo.c' ], u'fixes',
links=['http://buildbot.net'])
self.assertEqual(c.links, ['http://buildbot.net'])

def test_constructor_revision_int(self):
self.assertRaises(ValueError,
lambda : changes.Change(u'me', [ u'foo.c' ], u'fixes',
revision=456))

def test_constructor_revision_str(self):
self.assertRaises(ValueError,
lambda : changes.Change(u'me', [ u'foo.c' ], u'fixes',
revision='456'))

def test_constructor_revision_unicode(self):
c = changes.Change(u'me', [ u'foo.c' ], u'fixes',
revision=u'456')
self.assertEqualUnicode(c.revision, u'456')

def test_constructor_when(self):
c = changes.Change(u'me', [ u'foo.c' ], u'fixes',
when=1273727073)
self.assertEqual(c.when, 1273727073)

def test_constructor_branch_str(self):
self.assertRaises(ValueError,
lambda : changes.Change(u'me', [ u'foo.c' ], u'fixes',
branch='branches/release'))

def test_constructor_branch_unicode(self):
c = changes.Change(u'me', [ u'foo.c' ], u'fixes',
branch=u'branches/release')
self.assertEqualUnicode(c.branch, u'branches/release')

def test_constructor_category_str(self):
self.assertRaises(ValueError,
lambda : changes.Change(u'me', [ u'foo.c' ], u'fixes',
category='important'))

def test_constructor_category_unicode(self):
c = changes.Change(u'me', [ u'foo.c' ], u'fixes',
category=u'important')
self.assertEqualUnicode(c.category, u'important')

def test_constructor_revlink_str(self):
self.assertRaises(ValueError,
lambda : changes.Change(u'me', [ u'foo.c' ], u'fixes',
revlink='http://buildbot.net'))

def test_constructor_revlink_unicode(self):
c = changes.Change(u'me', [ u'foo.c' ], u'fixes',
revlink=u'http://buildbot.net')
self.assertEqualUnicode(c.revlink, u'http://buildbot.net')

def test_constructor_repository_str(self):
self.assertRaises(ValueError,
lambda : changes.Change(u'me', [ u'foo.c' ], u'fixes',
repository='http://svn.buildbot.net'))

def test_constructor_repository_unicode(self):
c = changes.Change(u'me', [ u'foo.c' ], u'fixes',
repository=u'http://svn.buildbot.net')
self.assertEqualUnicode(c.repository, u'http://svn.buildbot.net')

def test_constructor_project_str(self):
self.assertRaises(ValueError,
lambda : changes.Change(u'me', [ u'foo.c' ], u'fixes',
project='buildbot'))

def test_constructor_project_unicode(self):
c = changes.Change(u'me', [ u'foo.c' ], u'fixes',
project=u'buildbot')
self.assertEqualUnicode(c.project, u'buildbot')

def test_asDict(self):
c = self.getBigChange()
self.assertEqual(c.asDict(), dict(
number=None,
branch=u'branches/release',
category=u'important',
who=u'me',
comments=u'fixes',
revision=u'456',
rev=u'456',
when=1273727073,
at=time.strftime("%a %d %b %Y %H:%M:%S", time.localtime(1273727073)),
files=[dict(name=u'foo.c', url=None)],
revlink='http://buildbot.net',
properties=[('foo', 'bar', 'Change')],
repository=u'http://svn.buildbot.net',
project=u'buildbot'))

def test_getTime_None(self):
c = self.getBigChange()
self.assertEqual(c.getTime(),
time.strftime("%a %d %b %Y %H:%M:%S", time.localtime(1273727073)))

def test_getTimes(self):
c = self.getBigChange()
self.assertEqual(c.getTimes(), (1273727073, None))

def test_getText(self):
c = self.getBigChange()
self.assertEqual(c.getText(), [u'me'])

def test_getText_htmlunsafe(self):
c = changes.Change(u'>hi<', [ u'foo.c' ], u'fixes')
self.assertEqual(c.getText(), [u'&gt;hi&lt;'])

0 comments on commit bb64620

Please sign in to comment.