Test failures in BundleTester if TZ=UTC

Martin Pool mbp at canonical.com
Thu Aug 10 03:48:43 BST 2006


On  9 Aug 2006, John Arbash Meinel <john at arbash-meinel.com> wrote:
> Thanks for the report.
> 
> It turns out that the current bzr xml serializer does this:
> if rev.timezone:
>   root.set('timezone', str(rev.timezone))
> 
> Which means that if your timezone == UTC (thus offset == 0), then it
> *doesn't* set a timezone property in the XML.
> And when read back in, that leaves the timezone property == None rather
> than == 0.
> 
> However, the bundle reader reads it in and sets rev.timezone = 0. Hence
> what has been stored disagrees with what has just been read in.
> 
> So what is the best bugfixes? I think there are a few things:
> 
> 1) Change xml5._pack_revision to use:
>    if rev.timezone is not None:
>      root.set('timezone', str(rev.timezone))
> 2) We can't change the read side, because that would mean reading a
> revision, and writing it back out again would have different text.
> Otherwise we could change these lines:
> 
> OLD
>   v = elt.get('timezone')
>   rev.timezone = v and int(v)
> NEW
>   v = elt.get('timezone')
>   if v is None:
>     rev.timezone = 0
>   else:
>     rev.timezone = int(v)
> 
> 3) we could just change the tests so that they handle timezone==None for
> timezone == 0. I don't really like that one.
> 
> So really old revisions would have timezone == None, because when Martin
> first wrote bzr, he didn't set timezone.
> 
> However, testaments already do:
> self.timezone = rev.timezone or 0
> 
> Since we don't really assert that Revision XML text doesn't change (that
> is what testaments are for), I propose we do both (1) and (2) and write
> tests for these cases.

Well done.

I'm not sure what the "otherwise" means in #2 - that you will change
them?

#1 seems obviously correct.

It probably be OK to treat a missing field as UTC and so change the
reader along the lines ofo

> OLD
>   v = elt.get('timezone')
>   rev.timezone = v and int(v)
> NEW
>   v = elt.get('timezone')
>   if v is None:
>     rev.timezone = 0
>   else:
>     rev.timezone = int(v)

-- 
Martin




More information about the bazaar mailing list