You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jspwiki.apache.org by Andrew Jaquith <an...@gmail.com> on 2009/12/01 14:44:43 UTC
Failing DeleteActionBean tests
Janne, can I get a quick sanity check on something?
// Save two versions of the test page
m_engine.saveText( "Test", "This is the first version" );
m_engine.saveText( "Test", "This is the second version" );
// Make sure they both saved ok
WikiPage v1 = m_engine.getPage( "Test", 1 );
WikiPage v2 = m_engine.getPage( "Test", 2 );
assertNotNull( "Did not save page Test, v1!", v1 );
assertNotNull( "Did not save page Test, v2!", v2 );
assertEquals( "This is the first version",
m_engine.getPureText( v1 ).trim() );
The last assertion is failing. When WikiEngine.getPureText(v1) is
called, it is returning "This is the second version", which seems
wrong . It should return the first version, right?
Back when the trunk tests all ran clean (aka "the good old days"),
this test worked also. So it seems to me that some recent changes
caused the results to change. So was the test wrong all along, or is
there a bug somewhere?
Andrew
Re: Failing DeleteActionBean tests
Posted by Janne Jalkanen <Ja...@ecyrd.com>.
The fix is in the trunk now. Not everything is working yet; I'm seeing
oddities with page content (e.g. WikiengineTest.deleteVersion()), but
I think those are fairly easy to figure out now.
/Janne
On Dec 1, 2009, at 23:51 , Janne Jalkanen wrote:
>
> Got it. JCRWikiPage.getJCRNode() creates the page if it does not
> exist. This has the unfortunate side effect that *any* calls to
> WikiPage after it has been deleted do not result in exceptions, but
> they do result in a new Node being created, which in turn screws up
> quite a lot of stuff...
>
> The fun thing is, the commit was marked as "now all unit tests
> run" ;-)
>
> /Janne
>
> On Dec 1, 2009, at 19:59 , Andrew Jaquith wrote:
>
>> The easy way to test this hypothesis, which is an interesting one,
>> would be to unhook the ReferenceManager event listener and see if it
>> makes a difference. The RefMgr tests will fail, but perhaps some of
>> the other failing tests will succeed.
>>
>> Generally, though, we should definitely do one of two things: either
>> create a separate "updater" JCR session for the ReferenceManager
>> event
>> listener, and have it do its own saves, OR remove all save()s from
>> ReferenceManager entirely. Right now it does saves in the context of
>> the caller's JCR session, which doesn't feel right.
>>
>> Andrew
>>
>> On Tue, Dec 1, 2009 at 11:25 AM, Janne Jalkanen
>> <Ja...@ecyrd.com> wrote:
>>>
>>> The test seems correct. There seems to be something fairly odd
>>> going on
>>> with respect to versioning; for example,
>>> ContentManagerTest.deleteAllPages()
>>> fails because it deletes all pages ok, but at the last instant it
>>> recreates
>>> the pages as empty.
>>>
>>> My guess is that it was a bug in ContentManager that priha 0.6.x
>>> revealed;
>>> and I also have a nagging suspicion it has something to do with
>>> ReferenceManager, as it seems to me that the pages are recreated
>>> by the
>>> event handlers as a side effect. This could happen if the page is
>>> first
>>> deleted, then save()d, then a event handler adds a property to it
>>> and
>>> recreates the Node as a sanity check or something. Haven't yet had
>>> time to
>>> peek into it very deeply.
>>>
>>> /Janne
>>>
>>> On Dec 1, 2009, at 15:44 , Andrew Jaquith wrote:
>>>
>>>> Janne, can I get a quick sanity check on something?
>>>>
>>>> // Save two versions of the test page
>>>> m_engine.saveText( "Test", "This is the first version" );
>>>> m_engine.saveText( "Test", "This is the second version" );
>>>>
>>>> // Make sure they both saved ok
>>>> WikiPage v1 = m_engine.getPage( "Test", 1 );
>>>> WikiPage v2 = m_engine.getPage( "Test", 2 );
>>>> assertNotNull( "Did not save page Test, v1!", v1 );
>>>> assertNotNull( "Did not save page Test, v2!", v2 );
>>>> assertEquals( "This is the first version",
>>>> m_engine.getPureText( v1 ).trim() );
>>>>
>>>> The last assertion is failing. When WikiEngine.getPureText(v1) is
>>>> called, it is returning "This is the second version", which seems
>>>> wrong . It should return the first version, right?
>>>>
>>>> Back when the trunk tests all ran clean (aka "the good old days"),
>>>> this test worked also. So it seems to me that some recent changes
>>>> caused the results to change. So was the test wrong all along, or
>>>> is
>>>> there a bug somewhere?
>>>>
>>>> Andrew
>>>
>>>
Re: Failing DeleteActionBean tests
Posted by Janne Jalkanen <Ja...@ecyrd.com>.
Got it. JCRWikiPage.getJCRNode() creates the page if it does not
exist. This has the unfortunate side effect that *any* calls to
WikiPage after it has been deleted do not result in exceptions, but
they do result in a new Node being created, which in turn screws up
quite a lot of stuff...
The fun thing is, the commit was marked as "now all unit tests run" ;-)
/Janne
On Dec 1, 2009, at 19:59 , Andrew Jaquith wrote:
> The easy way to test this hypothesis, which is an interesting one,
> would be to unhook the ReferenceManager event listener and see if it
> makes a difference. The RefMgr tests will fail, but perhaps some of
> the other failing tests will succeed.
>
> Generally, though, we should definitely do one of two things: either
> create a separate "updater" JCR session for the ReferenceManager event
> listener, and have it do its own saves, OR remove all save()s from
> ReferenceManager entirely. Right now it does saves in the context of
> the caller's JCR session, which doesn't feel right.
>
> Andrew
>
> On Tue, Dec 1, 2009 at 11:25 AM, Janne Jalkanen
> <Ja...@ecyrd.com> wrote:
>>
>> The test seems correct. There seems to be something fairly odd
>> going on
>> with respect to versioning; for example,
>> ContentManagerTest.deleteAllPages()
>> fails because it deletes all pages ok, but at the last instant it
>> recreates
>> the pages as empty.
>>
>> My guess is that it was a bug in ContentManager that priha 0.6.x
>> revealed;
>> and I also have a nagging suspicion it has something to do with
>> ReferenceManager, as it seems to me that the pages are recreated by
>> the
>> event handlers as a side effect. This could happen if the page is
>> first
>> deleted, then save()d, then a event handler adds a property to it and
>> recreates the Node as a sanity check or something. Haven't yet had
>> time to
>> peek into it very deeply.
>>
>> /Janne
>>
>> On Dec 1, 2009, at 15:44 , Andrew Jaquith wrote:
>>
>>> Janne, can I get a quick sanity check on something?
>>>
>>> // Save two versions of the test page
>>> m_engine.saveText( "Test", "This is the first version" );
>>> m_engine.saveText( "Test", "This is the second version" );
>>>
>>> // Make sure they both saved ok
>>> WikiPage v1 = m_engine.getPage( "Test", 1 );
>>> WikiPage v2 = m_engine.getPage( "Test", 2 );
>>> assertNotNull( "Did not save page Test, v1!", v1 );
>>> assertNotNull( "Did not save page Test, v2!", v2 );
>>> assertEquals( "This is the first version",
>>> m_engine.getPureText( v1 ).trim() );
>>>
>>> The last assertion is failing. When WikiEngine.getPureText(v1) is
>>> called, it is returning "This is the second version", which seems
>>> wrong . It should return the first version, right?
>>>
>>> Back when the trunk tests all ran clean (aka "the good old days"),
>>> this test worked also. So it seems to me that some recent changes
>>> caused the results to change. So was the test wrong all along, or is
>>> there a bug somewhere?
>>>
>>> Andrew
>>
>>
Re: Failing DeleteActionBean tests
Posted by Andrew Jaquith <an...@gmail.com>.
The easy way to test this hypothesis, which is an interesting one,
would be to unhook the ReferenceManager event listener and see if it
makes a difference. The RefMgr tests will fail, but perhaps some of
the other failing tests will succeed.
Generally, though, we should definitely do one of two things: either
create a separate "updater" JCR session for the ReferenceManager event
listener, and have it do its own saves, OR remove all save()s from
ReferenceManager entirely. Right now it does saves in the context of
the caller's JCR session, which doesn't feel right.
Andrew
On Tue, Dec 1, 2009 at 11:25 AM, Janne Jalkanen
<Ja...@ecyrd.com> wrote:
>
> The test seems correct. There seems to be something fairly odd going on
> with respect to versioning; for example, ContentManagerTest.deleteAllPages()
> fails because it deletes all pages ok, but at the last instant it recreates
> the pages as empty.
>
> My guess is that it was a bug in ContentManager that priha 0.6.x revealed;
> and I also have a nagging suspicion it has something to do with
> ReferenceManager, as it seems to me that the pages are recreated by the
> event handlers as a side effect. This could happen if the page is first
> deleted, then save()d, then a event handler adds a property to it and
> recreates the Node as a sanity check or something. Haven't yet had time to
> peek into it very deeply.
>
> /Janne
>
> On Dec 1, 2009, at 15:44 , Andrew Jaquith wrote:
>
>> Janne, can I get a quick sanity check on something?
>>
>> // Save two versions of the test page
>> m_engine.saveText( "Test", "This is the first version" );
>> m_engine.saveText( "Test", "This is the second version" );
>>
>> // Make sure they both saved ok
>> WikiPage v1 = m_engine.getPage( "Test", 1 );
>> WikiPage v2 = m_engine.getPage( "Test", 2 );
>> assertNotNull( "Did not save page Test, v1!", v1 );
>> assertNotNull( "Did not save page Test, v2!", v2 );
>> assertEquals( "This is the first version",
>> m_engine.getPureText( v1 ).trim() );
>>
>> The last assertion is failing. When WikiEngine.getPureText(v1) is
>> called, it is returning "This is the second version", which seems
>> wrong . It should return the first version, right?
>>
>> Back when the trunk tests all ran clean (aka "the good old days"),
>> this test worked also. So it seems to me that some recent changes
>> caused the results to change. So was the test wrong all along, or is
>> there a bug somewhere?
>>
>> Andrew
>
>
Re: Failing DeleteActionBean tests
Posted by Janne Jalkanen <Ja...@ecyrd.com>.
The test seems correct. There seems to be something fairly odd going
on with respect to versioning; for example,
ContentManagerTest.deleteAllPages() fails because it deletes all pages
ok, but at the last instant it recreates the pages as empty.
My guess is that it was a bug in ContentManager that priha 0.6.x
revealed; and I also have a nagging suspicion it has something to do
with ReferenceManager, as it seems to me that the pages are recreated
by the event handlers as a side effect. This could happen if the page
is first deleted, then save()d, then a event handler adds a property
to it and recreates the Node as a sanity check or something. Haven't
yet had time to peek into it very deeply.
/Janne
On Dec 1, 2009, at 15:44 , Andrew Jaquith wrote:
> Janne, can I get a quick sanity check on something?
>
> // Save two versions of the test page
> m_engine.saveText( "Test", "This is the first version" );
> m_engine.saveText( "Test", "This is the second version" );
>
> // Make sure they both saved ok
> WikiPage v1 = m_engine.getPage( "Test", 1 );
> WikiPage v2 = m_engine.getPage( "Test", 2 );
> assertNotNull( "Did not save page Test, v1!", v1 );
> assertNotNull( "Did not save page Test, v2!", v2 );
> assertEquals( "This is the first version",
> m_engine.getPureText( v1 ).trim() );
>
> The last assertion is failing. When WikiEngine.getPureText(v1) is
> called, it is returning "This is the second version", which seems
> wrong . It should return the first version, right?
>
> Back when the trunk tests all ran clean (aka "the good old days"),
> this test worked also. So it seems to me that some recent changes
> caused the results to change. So was the test wrong all along, or is
> there a bug somewhere?
>
> Andrew