Storm

Merge lp:~therve/storm/277095-update-cache into lp:storm

  1. 277095-update-cache
  2. Merge into trunk
Proposed by Thomas Herve
Status: Merged
Merge reported by: Thomas Herve
Merged at revision: 271
Proposed branch: lp:~therve/storm/277095-update-cache
Merge into: lp:storm
To merge this branch: bzr merge lp:~therve/storm/277095-update-cache
Reviewer Review Type Date Requested Status
Gustavo Niemeyer Approve
James Henstridge Approve
Review via email: mp+1200@code.launchpad.net
To post a comment you must log in.
Revision history for this message
James Henstridge (jamesh) wrote :

Looks good. You could probably simplify the test a bit: I don't think you need to overlap the two transactions to trigger the bug. The following fails without your changes and passes with your changes, even on sqlite:

        store = self.create_store()
        foo2 = store.get(Foo, 10)
        self.assertEquals(foo2.title, u"Title 30")
        store.commit()

        foo1 = self.store.get(Foo, 10)
        foo1.title = u"Title 40"
        self.store.commit()

        foo2.title = u"Title 30"
        store.commit()
        self.assertEquals(foo2.title, u"Title 30")

Not having to disable the test on sqlite seems worth doing.

review: Approve
Revision history for this message
Thomas Herve (therve) wrote :

Thanks a lot, change applied.

lp:~therve/storm/277095-update-cache updated
272. By Thomas Herve

Reorder the test case to make it pass under SQLite (from James).

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

Looks great! +1!

review: Approve
lp:~therve/storm/277095-update-cache updated
273. By Thomas Herve

Remove trailing whitespaces

Subscribers

People subscribed via source and target branches

to status/vote changes: